Re: [Nouveau] [PATCH v4 22/37] clk: rename nvkm_pstate_calc to nvkm_clk_update

2016-04-20 Thread Martin Peres 

On 18/04/16 22:13, Karol Herbst wrote:

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

This will happen for various reasons:
  * temperature changes (may change cstate and/or voltage)
  * user changes boost mode
  * load changes

v2: add wait parameter

Signed-off-by: Karol Herbst 
---
  drm/nouveau/include/nvkm/subdev/clk.h |  1 +
  drm/nouveau/nvkm/subdev/clk/base.c| 46 ---
  2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/clk.h 
b/drm/nouveau/include/nvkm/subdev/clk.h
index 61d99fd..77d94c1 100644
--- a/drm/nouveau/include/nvkm/subdev/clk.h
+++ b/drm/nouveau/include/nvkm/subdev/clk.h
@@ -120,6 +120,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 *, int req, int rel);
+int nvkm_clk_update(struct nvkm_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 1ca25dd..bfc6a49 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -274,11 +274,14 @@ static int
  nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
  {
struct nvkm_subdev *subdev = >subdev;
-   struct nvkm_ram *ram = subdev->device->fb->ram;
+   struct nvkm_fb *fb = subdev->device->fb;
struct nvkm_pci *pci = subdev->device->pci;
struct nvkm_pstate *pstate;
int ret, idx = 0;
  
+	if (pstatei == -1)

+   return 0;
+
list_for_each_entry(pstate, >states, head) {
if (idx++ == pstatei)
break;
@@ -289,7 +292,8 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
  
  	nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
  
-	if (ram && ram->func->calc) {

+   if (fb && fb->ram && fb->ram->func->calc) {
+   struct nvkm_ram *ram = fb->ram;
int khz = pstate->base.domain[nv_clk_src_mem];
do {
ret = ram->func->calc(ram, khz);


All the changes to this file do not belong to this patch. What are you 
doing here anyway?

@@ -303,11 +307,11 @@ 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 = >subdev;
-   int pstate;
+   int pstate, ret;
  
  	if (!atomic_xchg(>waiting, 0))

return;
@@ -327,21 +331,25 @@ nvkm_pstate_work(struct work_struct *work)
}
  
  	nvkm_trace(subdev, "-> %d\n", pstate);

-   if (pstate != clk->pstate) {
-   int ret = nvkm_pstate_prog(clk, pstate);
-   if (ret) {
-   nvkm_error(subdev, "error setting pstate %d: %d\n",
-  pstate, ret);
-   }
+   ret = nvkm_pstate_prog(clk, pstate);
+   if (ret) {
+   nvkm_error(subdev, "error setting pstate %d: %d\n",
+  pstate, ret);
}

Why did you get rid of the if?

  
  	wake_up_all(>wait);

nvkm_notify_get(>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;


This is also not part of the rename. Why do you meed this suddenly?

+
atomic_set(>waiting, 1);
schedule_work(>work);
if (wait)
@@ -531,7 +539,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;
  }
@@ -543,7 +551,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

@@ -553,7 +561,7 @@ nvkm_clk_tstate(struct nvkm_clk *clk, int req, int rel)
if ( rel) clk->tstate += rel;
clk->tstate = min(clk->tstate, 0);
clk->tstate = max(clk->tstate, -(clk->state_nr - 1));
-   return nvkm_pstate_calc(clk, true);
+   return nvkm_clk_update(clk, true);
  }
  
  int

@@ -563,7 +571,7 @@ nvkm_clk_dstate(struct nvkm_clk *clk, int req, int rel)
if (

[Nouveau] [PATCH v4 22/37] clk: rename nvkm_pstate_calc to nvkm_clk_update

2016-04-18 Thread Karol Herbst 
this function will be used to update the current clock state.

This will happen for various reasons:
 * temperature changes (may change cstate and/or voltage)
 * user changes boost mode
 * load changes

v2: add wait parameter

Signed-off-by: Karol Herbst 
---
 drm/nouveau/include/nvkm/subdev/clk.h |  1 +
 drm/nouveau/nvkm/subdev/clk/base.c| 46 ---
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/drm/nouveau/include/nvkm/subdev/clk.h 
b/drm/nouveau/include/nvkm/subdev/clk.h
index 61d99fd..77d94c1 100644
--- a/drm/nouveau/include/nvkm/subdev/clk.h
+++ b/drm/nouveau/include/nvkm/subdev/clk.h
@@ -120,6 +120,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 *, int req, int rel);
+int nvkm_clk_update(struct nvkm_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 1ca25dd..bfc6a49 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -274,11 +274,14 @@ static int
 nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
 {
struct nvkm_subdev *subdev = >subdev;
-   struct nvkm_ram *ram = subdev->device->fb->ram;
+   struct nvkm_fb *fb = subdev->device->fb;
struct nvkm_pci *pci = subdev->device->pci;
struct nvkm_pstate *pstate;
int ret, idx = 0;
 
+   if (pstatei == -1)
+   return 0;
+
list_for_each_entry(pstate, >states, head) {
if (idx++ == pstatei)
break;
@@ -289,7 +292,8 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstatei)
 
nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width);
 
-   if (ram && ram->func->calc) {
+   if (fb && fb->ram && fb->ram->func->calc) {
+   struct nvkm_ram *ram = fb->ram;
int khz = pstate->base.domain[nv_clk_src_mem];
do {
ret = ram->func->calc(ram, khz);
@@ -303,11 +307,11 @@ 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 = >subdev;
-   int pstate;
+   int pstate, ret;
 
if (!atomic_xchg(>waiting, 0))
return;
@@ -327,21 +331,25 @@ nvkm_pstate_work(struct work_struct *work)
}
 
nvkm_trace(subdev, "-> %d\n", pstate);
-   if (pstate != clk->pstate) {
-   int ret = nvkm_pstate_prog(clk, pstate);
-   if (ret) {
-   nvkm_error(subdev, "error setting pstate %d: %d\n",
-  pstate, ret);
-   }
+   ret = nvkm_pstate_prog(clk, pstate);
+   if (ret) {
+   nvkm_error(subdev, "error setting pstate %d: %d\n",
+  pstate, ret);
}
 
wake_up_all(>wait);
nvkm_notify_get(>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(>waiting, 1);
schedule_work(>work);
if (wait)
@@ -531,7 +539,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;
 }
@@ -543,7 +551,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
@@ -553,7 +561,7 @@ nvkm_clk_tstate(struct nvkm_clk *clk, int req, int rel)
if ( rel) clk->tstate += rel;
clk->tstate = min(clk->tstate, 0);
clk->tstate = max(clk->tstate, -(clk->state_nr - 1));
-   return nvkm_pstate_calc(clk, true);
+   return nvkm_clk_update(clk, true);
 }
 
 int
@@ -563,7 +571,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
@@ -571,7 +579,7 @@