[v2,07/22] volt: add min_id parameter to nvkm_volt_set_id

Submitted by Karol Herbst on March 21, 2016, 4:16 p.m.

Details

Message ID 1458577000-6615-8-git-send-email-nouveau@karolherbst.de
State New
Headers show
Series "Volting/Clocking improvements for Fermi and newer" ( rev: 2 ) in Nouveau

Not browsing as part of any series.

Commit Message

Karol Herbst March 21, 2016, 4:16 p.m.
min_id indicates a volt map entry which acts as a floor value, this will be
used to set the lower voltage limit through pstates

Signed-off-by: Karol Herbst <nouveau@karolherbst.de>
---
 drm/nouveau/include/nvkm/subdev/volt.h | 2 +-
 drm/nouveau/nvkm/subdev/clk/base.c     | 6 ++++--
 drm/nouveau/nvkm/subdev/volt/base.c    | 5 ++++-
 3 files changed, 9 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drm/nouveau/include/nvkm/subdev/volt.h b/drm/nouveau/include/nvkm/subdev/volt.h
index 3e0f8da..e966266 100644
--- a/drm/nouveau/include/nvkm/subdev/volt.h
+++ b/drm/nouveau/include/nvkm/subdev/volt.h
@@ -21,7 +21,7 @@  struct nvkm_volt {
 
 int nvkm_volt_map_min(struct nvkm_volt *volt, u8 id);
 int nvkm_volt_get(struct nvkm_volt *);
-int nvkm_volt_set_id(struct nvkm_volt *, u8 id, int condition);
+int nvkm_volt_set_id(struct nvkm_volt *, u8 id, u8 min_id, int condition);
 
 int nv40_volt_new(struct nvkm_device *, int, struct nvkm_volt **);
 int gk104_volt_new(struct nvkm_device *, int, struct nvkm_volt **);
diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c
index 75122a2..ee193cf 100644
--- a/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drm/nouveau/nvkm/subdev/clk/base.c
@@ -99,7 +99,8 @@  nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei)
 	}
 
 	if (volt) {
-		ret = nvkm_volt_set_id(volt, cstate->voltage, +1);
+		ret = nvkm_volt_set_id(volt, cstate->voltage,
+				       pstate->base.voltage, +1);
 		if (ret && ret != -ENODEV) {
 			nvkm_error(subdev, "failed to raise voltage: %d\n", ret);
 			return ret;
@@ -113,7 +114,8 @@  nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei)
 	}
 
 	if (volt) {
-		ret = nvkm_volt_set_id(volt, cstate->voltage, -1);
+		ret = nvkm_volt_set_id(volt, cstate->voltage,
+				       pstate->base.voltage, -1);
 		if (ret && ret != -ENODEV)
 			nvkm_error(subdev, "failed to lower voltage: %d\n", ret);
 	}
diff --git a/drm/nouveau/nvkm/subdev/volt/base.c b/drm/nouveau/nvkm/subdev/volt/base.c
index 205dfcf..a7f8c76 100644
--- a/drm/nouveau/nvkm/subdev/volt/base.c
+++ b/drm/nouveau/nvkm/subdev/volt/base.c
@@ -110,7 +110,7 @@  nvkm_volt_map(struct nvkm_volt *volt, u8 id)
 }
 
 int
-nvkm_volt_set_id(struct nvkm_volt *volt, u8 id, int condition)
+nvkm_volt_set_id(struct nvkm_volt *volt, u8 id, u8 min_id, int condition)
 {
 	int ret;
 
@@ -123,6 +123,9 @@  nvkm_volt_set_id(struct nvkm_volt *volt, u8 id, int condition)
 		if (!condition || prev < 0 ||
 		    (condition < 0 && ret < prev) ||
 		    (condition > 0 && ret > prev)) {
+			int min = nvkm_volt_map(volt, min_id);
+			if (min >= 0)
+				ret = max(min, ret);
 			ret = nvkm_volt_set(volt, ret);
 		} else {
 			ret = 0;

Comments

On 21/03/16 18:16, Karol Herbst wrote:
> min_id indicates a volt map entry which acts as a floor value, this will be
> used to set the lower voltage limit through pstates
>
> Signed-off-by: Karol Herbst <nouveau@karolherbst.de>

Do we really want to push reclocking logic to the volt subsystem?

To me, volt should just allow you to read back and set a voltage. All 
the rest of the logic should be in clk.

Since this is my first NAK, here are my R-b for 3, 4 and 5:

Reviewed-by: Martin Peres <martin.peres@free.fr>
On 28/03/16 23:52, Martin Peres wrote:
> On 21/03/16 18:16, Karol Herbst wrote:
>> min_id indicates a volt map entry which acts as a floor value, this will be
>> used to set the lower voltage limit through pstates

Please state that this comes that this min_id is different for each 
pstate, hence why volt should not know about this and needs to take it 
as an input!

>>
>> Signed-off-by: Karol Herbst <nouveau@karolherbst.de>
> Do we really want to push reclocking logic to the volt subsystem?
>
> To me, volt should just allow you to read back and set a voltage. All
> the rest of the logic should be in clk.
>
> Since this is my first NAK, here are my R-b for 3, 4 and 5:
>
> Reviewed-by: Martin Peres <martin.peres@free.fr>

With this fixed, this patch is Reviewed-by: Martin Peres 
<martin.peres@free.fr>