Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 00/29] Current State of my clk patches
Just wanted to post updated versions of my last series/patches. Reviews welcomed. It would be also nice if we agree on features I should focus upstreaming, so that this work can be better splitted or reordered. Sadly most of my patches depend on the rather big clk subdev rework and I think those patches shows best, why I think this rework is actually needed and makes things much easier to add later on. Feature overview: 1- 3: Hwmon fixes on suspended device 4- 8: Clk subdev rework to decouple current and expected clock states Fix reclocking while suspended 9: Restore clocks on Suspend 10: Fix reclocking while going into suspend 11: Fix reclocking while suspending 12-14: Thermal daemon triggers reclocks on temperature change when needed includes volt updates and dropping cstates according to the vbios 15-17: Thermal throttling 18: Don't do full reclocks when only parts need to be updated 19-20: Hacky Workaround for enabling Maxwell2 reclocking (hidden behind module parameter) 21-23: debugfs file to change boost mode 24-29: Parse battery vpstate to throttle clocks when system is running on battery Karol Herbst (29): therm: split return code and value in nvkm_get_temp hwmon: properly check for errors subdev/volt/gk104: return error when read fails 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: Set clocks to pre suspend state after suspend core/device: Move therm behind clk debugfs: Wake up GPU before doing any reclocking therm: Don't cancel the timer therm: Move the temp readout into the alarm therm: Trigger reclock in temperature daemon bios: add thermal policies table clk: parse thermal policies for throttling thresholds clk: thermal throttling clk: Only do partial reclocks as required secboot/acr352: reset PMU after secboot device: enable clk for Maxwell2 clk: Save the max clock we can set nvif: Add boost info and set operations debugfs: Add boost interface to change the boost_mode bios/vpstate: there are some fermi vbios with no boost or tdp entry bios/vpstate: parse max battery id clk: refactor the base and boost clock limits so that we can limit pstates as well clk: implement limiting pstates just like we do for cstates clk: move the switch out of the loop in nvkm_cstate_valid clk: limit clocks on battery drm/nouveau/include/nvif/if0001.h | 15 + drm/nouveau/include/nvkm/core/device.h | 2 +- .../include/nvkm/subdev/bios/thermal_policies.h | 27 ++ drm/nouveau/include/nvkm/subdev/bios/vpstate.h | 1 + drm/nouveau/include/nvkm/subdev/clk.h | 23 +- drm/nouveau/include/nvkm/subdev/therm.h | 3 +- drm/nouveau/nouveau_debugfs.c | 90 ++++- drm/nouveau/nouveau_hwmon.c | 48 ++- drm/nouveau/nvkm/engine/device/base.c | 3 + drm/nouveau/nvkm/engine/device/ctrl.c | 66 +++- drm/nouveau/nvkm/subdev/bios/Kbuild | 1 + drm/nouveau/nvkm/subdev/bios/thermal_policies.c | 81 +++++ drm/nouveau/nvkm/subdev/bios/vpstate.c | 13 +- drm/nouveau/nvkm/subdev/clk/base.c | 375 +++++++++++++++------ drm/nouveau/nvkm/subdev/clk/gk104.c | 10 +- drm/nouveau/nvkm/subdev/pmu/gk20a.c | 18 +- drm/nouveau/nvkm/subdev/secboot/acr_r352.c | 13 + drm/nouveau/nvkm/subdev/therm/base.c | 59 ++-- drm/nouveau/nvkm/subdev/therm/g84.c | 11 +- drm/nouveau/nvkm/subdev/therm/gp100.c | 9 +- drm/nouveau/nvkm/subdev/therm/nv40.c | 9 +- drm/nouveau/nvkm/subdev/therm/nv50.c | 9 +- drm/nouveau/nvkm/subdev/therm/priv.h | 4 +- drm/nouveau/nvkm/subdev/therm/temp.c | 16 +- drm/nouveau/nvkm/subdev/volt/base.c | 3 + drm/nouveau/nvkm/subdev/volt/gk104.c | 7 +- 26 files changed, 723 insertions(+), 193 deletions(-) create mode 100644 drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h create mode 100644 drm/nouveau/nvkm/subdev/bios/thermal_policies.c -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 01/29] therm: split return code and value in nvkm_get_temp
The current hwmon code doesn't check if the returned value was actually an error. Since Kepler temperature sensors are able to report negative values. Since Pascal (and maybe earlier) we have sensors with improved precision. Adjust the nvkm_get_temp method to be able to deal with those changes and let hwmon return an error properly. Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/subdev/therm.h | 2 +- drm/nouveau/nouveau_hwmon.c | 15 +++++++++------ drm/nouveau/nvkm/subdev/therm/base.c | 19 ++++++++++++++----- drm/nouveau/nvkm/subdev/therm/g84.c | 11 ++++++----- drm/nouveau/nvkm/subdev/therm/gp100.c | 9 +++++---- drm/nouveau/nvkm/subdev/therm/nv40.c | 9 +++------ drm/nouveau/nvkm/subdev/therm/nv50.c | 9 +++------ drm/nouveau/nvkm/subdev/therm/priv.h | 4 ++-- drm/nouveau/nvkm/subdev/therm/temp.c | 16 ++++++++++++---- 9 files changed, 55 insertions(+), 39 deletions(-) diff --git a/drm/nouveau/include/nvkm/subdev/therm.h b/drm/nouveau/include/nvkm/subdev/therm.h index 9841f076..8c84017f 100644 --- a/drm/nouveau/include/nvkm/subdev/therm.h +++ b/drm/nouveau/include/nvkm/subdev/therm.h @@ -86,7 +86,7 @@ struct nvkm_therm { int (*attr_set)(struct nvkm_therm *, enum nvkm_therm_attr_type, int); }; -int nvkm_therm_temp_get(struct nvkm_therm *); +int nvkm_therm_temp_get(struct nvkm_therm *, int *); int nvkm_therm_fan_sense(struct nvkm_therm *); int nvkm_therm_cstate(struct nvkm_therm *, int, int); diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c index 7c965648..f1914d9a 100644 --- a/drm/nouveau/nouveau_hwmon.c +++ b/drm/nouveau/nouveau_hwmon.c @@ -326,8 +326,9 @@ nouveau_temp_is_visible(const void *data, u32 attr, int channel) { struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data); struct nvkm_therm *therm = nvxx_therm(&drm->client.device); + int val; - if (therm && therm->attr_get && nvkm_therm_temp_get(therm) < 0) + if (therm && therm->attr_get && nvkm_therm_temp_get(therm, &val) < 0) return 0; switch (attr) { @@ -421,15 +422,16 @@ nouveau_temp_read(struct device *dev, u32 attr, int channel, long *val) struct drm_device *drm_dev = dev_get_drvdata(dev); struct nouveau_drm *drm = nouveau_drm(drm_dev); struct nvkm_therm *therm = nvxx_therm(&drm->client.device); - int ret; + int ret = 0; + int temp; if (!therm || !therm->attr_get) return -EOPNOTSUPP; switch (attr) { case hwmon_temp_input: - ret = nvkm_therm_temp_get(therm); - *val = ret < 0 ? ret : (ret * 1000); + ret = nvkm_therm_temp_get(therm, &temp); + *val = temp * 1000; break; case hwmon_temp_max: *val = therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK) @@ -459,7 +461,7 @@ nouveau_temp_read(struct device *dev, u32 attr, int channel, long *val) return -EOPNOTSUPP; } - return 0; + return ret; } static int @@ -713,6 +715,7 @@ nouveau_hwmon_init(struct drm_device *dev) struct device *hwmon_dev; int ret = 0; int i = 0; + int val; hwmon = drm->hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL); if (!hwmon) @@ -720,7 +723,7 @@ nouveau_hwmon_init(struct drm_device *dev) hwmon->dev = dev; if (therm && therm->attr_get && therm->attr_set) { - if (nvkm_therm_temp_get(therm) >= 0) + if (nvkm_therm_temp_get(therm, &val) >= 0) special_groups[i++] = &temp1_auto_point_sensor_group; if (therm->fan_get && therm->fan_get(therm) >= 0) special_groups[i++] = &pwm_fan_sensor_group; diff --git a/drm/nouveau/nvkm/subdev/therm/base.c b/drm/nouveau/nvkm/subdev/therm/base.c index f27fc6d0..8fa691fb 100644 --- a/drm/nouveau/nvkm/subdev/therm/base.c +++ b/drm/nouveau/nvkm/subdev/therm/base.c @@ -24,22 +24,26 @@ #include "priv.h" int -nvkm_therm_temp_get(struct nvkm_therm *therm) +nvkm_therm_temp_get(struct nvkm_therm *therm, int *val) { if (therm->func->temp_get) - return therm->func->temp_get(therm); + return therm->func->temp_get(therm, val); return -ENODEV; } static int nvkm_therm_update_trip(struct nvkm_therm *therm) { + int temp, ret; struct nvbios_therm_trip_point *trip = therm->fan->bios.trip, *cur_trip = NULL, *last_trip = therm->last_trip; - u8 temp = therm->func->temp_get(therm); u16 duty, i; + ret = therm->func->temp_get(therm, &temp); + if (ret < 0) + return ret; + /* look for the trip point corresponding to the current temperature */ cur_trip = NULL; for (i = 0; i < therm->fan->bios.nr_fan_trip; i++) { @@ -67,9 +71,13 @@ static int nvkm_therm_compute_linear_duty(struct nvkm_therm *therm, u8 linear_min_temp, u8 linear_max_temp) { - u8 temp = therm->func->temp_get(therm); + int temp, ret; u16 duty; + ret = therm->func->temp_get(therm, &temp); + if (ret < 0) + return ret; + /* handle the non-linear part first */ if (temp < linear_min_temp) return therm->fan->bios.min_duty; @@ -182,6 +190,7 @@ nvkm_therm_fan_mode(struct nvkm_therm *therm, int mode) { struct nvkm_subdev *subdev = &therm->subdev; struct nvkm_device *device = subdev->device; + int val; static const char *name[] = { "disabled", "manual", @@ -197,7 +206,7 @@ nvkm_therm_fan_mode(struct nvkm_therm *therm, int mode) /* do not allow automatic fan management if the thermal sensor is * not available */ if (mode == NVKM_THERM_CTRL_AUTO && - therm->func->temp_get(therm) < 0) + therm->func->temp_get(therm, &val) < 0) return -EINVAL; if (therm->mode == mode) diff --git a/drm/nouveau/nvkm/subdev/therm/g84.c b/drm/nouveau/nvkm/subdev/therm/g84.c index 96f8da40..3bb2d829 100644 --- a/drm/nouveau/nvkm/subdev/therm/g84.c +++ b/drm/nouveau/nvkm/subdev/therm/g84.c @@ -27,14 +27,15 @@ #include <subdev/fuse.h> int -g84_temp_get(struct nvkm_therm *therm) +g84_temp_get(struct nvkm_therm *therm, int *val) { struct nvkm_device *device = therm->subdev.device; - if (nvkm_fuse_read(device->fuse, 0x1a8) == 1) - return nvkm_rd32(device, 0x20400); - else + if (nvkm_fuse_read(device->fuse, 0x1a8) != 1) return -ENODEV; + + *val = nvkm_rd32(device, 0x20400); + return 0; } void @@ -115,7 +116,7 @@ g84_therm_threshold_hyst_emulation(struct nvkm_therm *therm, } /* fix the state (in case someone reprogrammed the alarms) */ - cur = therm->func->temp_get(therm); + therm->func->temp_get(therm, &cur); if (new_state == NVKM_THERM_THRS_LOWER && cur > thrs->temp) new_state = NVKM_THERM_THRS_HIGHER; else if (new_state == NVKM_THERM_THRS_HIGHER && diff --git a/drm/nouveau/nvkm/subdev/therm/gp100.c b/drm/nouveau/nvkm/subdev/therm/gp100.c index 9f0dea3f..d8206748 100644 --- a/drm/nouveau/nvkm/subdev/therm/gp100.c +++ b/drm/nouveau/nvkm/subdev/therm/gp100.c @@ -24,7 +24,7 @@ #include "priv.h" static int -gp100_temp_get(struct nvkm_therm *therm) +gp100_temp_get(struct nvkm_therm *therm, int *val) { struct nvkm_device *device = therm->subdev.device; struct nvkm_subdev *subdev = &therm->subdev; @@ -36,9 +36,10 @@ gp100_temp_get(struct nvkm_therm *therm) nvkm_trace(subdev, "reading temperature from SHADOWed sensor\n"); /* device valid */ - if (tsensor & 0x20000000) - return (inttemp >> 8); - else + if (tsensor & 0x20000000) { + *val = inttemp >> 8; + return 0; + } else return -ENODEV; } diff --git a/drm/nouveau/nvkm/subdev/therm/nv40.c b/drm/nouveau/nvkm/subdev/therm/nv40.c index 2c92ffb5..cfd5b215 100644 --- a/drm/nouveau/nvkm/subdev/therm/nv40.c +++ b/drm/nouveau/nvkm/subdev/therm/nv40.c @@ -70,7 +70,7 @@ nv40_sensor_setup(struct nvkm_therm *therm) } static int -nv40_temp_get(struct nvkm_therm *therm) +nv40_temp_get(struct nvkm_therm *therm, int *val) { struct nvkm_device *device = therm->subdev.device; struct nvbios_therm_sensor *sensor = &therm->bios_sensor; @@ -95,11 +95,8 @@ nv40_temp_get(struct nvkm_therm *therm) core_temp = core_temp + sensor->offset_num / sensor->offset_den; core_temp = core_temp + sensor->offset_constant - 8; - /* reserve negative temperatures for errors */ - if (core_temp < 0) - core_temp = 0; - - return core_temp; + *val = core_temp; + return 0; } static int diff --git a/drm/nouveau/nvkm/subdev/therm/nv50.c b/drm/nouveau/nvkm/subdev/therm/nv50.c index 9b57b433..62ec4063 100644 --- a/drm/nouveau/nvkm/subdev/therm/nv50.c +++ b/drm/nouveau/nvkm/subdev/therm/nv50.c @@ -126,7 +126,7 @@ nv50_sensor_setup(struct nvkm_therm *therm) } static int -nv50_temp_get(struct nvkm_therm *therm) +nv50_temp_get(struct nvkm_therm *therm, int *val) { struct nvkm_device *device = therm->subdev.device; struct nvbios_therm_sensor *sensor = &therm->bios_sensor; @@ -143,11 +143,8 @@ nv50_temp_get(struct nvkm_therm *therm) core_temp = core_temp + sensor->offset_num / sensor->offset_den; core_temp = core_temp + sensor->offset_constant - 8; - /* reserve negative temperatures for errors */ - if (core_temp < 0) - core_temp = 0; - - return core_temp; + *val = core_temp; + return 0; } static void diff --git a/drm/nouveau/nvkm/subdev/therm/priv.h b/drm/nouveau/nvkm/subdev/therm/priv.h index 1f46e371..b325ec5f 100644 --- a/drm/nouveau/nvkm/subdev/therm/priv.h +++ b/drm/nouveau/nvkm/subdev/therm/priv.h @@ -91,7 +91,7 @@ struct nvkm_therm_func { int (*pwm_set)(struct nvkm_therm *, int line, u32, u32); int (*pwm_clock)(struct nvkm_therm *, int line); - int (*temp_get)(struct nvkm_therm *); + int (*temp_get)(struct nvkm_therm *, int *); int (*fan_sense)(struct nvkm_therm *); @@ -105,7 +105,7 @@ int nv50_fan_pwm_get(struct nvkm_therm *, int, u32 *, u32 *); int nv50_fan_pwm_set(struct nvkm_therm *, int, u32, u32); int nv50_fan_pwm_clock(struct nvkm_therm *, int); -int g84_temp_get(struct nvkm_therm *); +int g84_temp_get(struct nvkm_therm *, int *); void g84_sensor_setup(struct nvkm_therm *); void g84_therm_fini(struct nvkm_therm *); diff --git a/drm/nouveau/nvkm/subdev/therm/temp.c b/drm/nouveau/nvkm/subdev/therm/temp.c index ddb2b2c6..5ec2dfb3 100644 --- a/drm/nouveau/nvkm/subdev/therm/temp.c +++ b/drm/nouveau/nvkm/subdev/therm/temp.c @@ -86,7 +86,10 @@ nvkm_therm_sensor_event(struct nvkm_therm *therm, enum nvkm_therm_thrs thrs, static const char * const thresholds[] = { "fanboost", "downclock", "critical", "shutdown" }; - int temperature = therm->func->temp_get(therm); + int temperature; + + if (therm->func->temp_get(therm, &temperature) < 0) + return; if (thrs < 0 || thrs > 3) return; @@ -140,7 +143,10 @@ nvkm_therm_threshold_hyst_polling(struct nvkm_therm *therm, { enum nvkm_therm_thrs_direction direction; enum nvkm_therm_thrs_state prev_state, new_state; - int temp = therm->func->temp_get(therm); + int temp; + + if (therm->func->temp_get(therm, &temp) < 0) + return; prev_state = nvkm_therm_sensor_get_threshold_state(therm, thrs_name); @@ -166,6 +172,7 @@ alarm_timer_callback(struct nvkm_alarm *alarm) struct nvbios_therm_sensor *sensor = &therm->bios_sensor; struct nvkm_timer *tmr = therm->subdev.device->timer; unsigned long flags; + int val; spin_lock_irqsave(&therm->sensor.alarm_program_lock, flags); @@ -185,7 +192,7 @@ alarm_timer_callback(struct nvkm_alarm *alarm) spin_unlock_irqrestore(&therm->sensor.alarm_program_lock, flags); /* schedule the next poll in one second */ - if (therm->func->temp_get(therm) >= 0) + if (therm->func->temp_get(therm, &val) >= 0) nvkm_timer_alarm(tmr, 1000000000ULL, alarm); } @@ -227,9 +234,10 @@ nvkm_therm_sensor_fini(struct nvkm_therm *therm, bool suspend) void nvkm_therm_sensor_preinit(struct nvkm_therm *therm) { + int val; const char *sensor_avail = "yes"; - if (therm->func->temp_get(therm) < 0) + if (therm->func->temp_get(therm, &val) < 0) sensor_avail = "no"; nvkm_debug(&therm->subdev, "internal sensor: %s\n", sensor_avail); -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 02/29] hwmon: properly check for errors
Otherwise hwmon interprets error codes as real values. Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nouveau_hwmon.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c index f1914d9a..0d75c14d 100644 --- a/drm/nouveau/nouveau_hwmon.c +++ b/drm/nouveau/nouveau_hwmon.c @@ -470,18 +470,23 @@ nouveau_fan_read(struct device *dev, u32 attr, int channel, long *val) struct drm_device *drm_dev = dev_get_drvdata(dev); struct nouveau_drm *drm = nouveau_drm(drm_dev); struct nvkm_therm *therm = nvxx_therm(&drm->client.device); + int ret; if (!therm) return -EOPNOTSUPP; switch (attr) { case hwmon_fan_input: - *val = nvkm_therm_fan_sense(therm); + ret = nvkm_therm_fan_sense(therm); break; default: return -EOPNOTSUPP; } + if (ret < 0) + return ret; + + *val = ret; return 0; } @@ -491,7 +496,7 @@ nouveau_in_read(struct device *dev, u32 attr, int channel, long *val) struct drm_device *drm_dev = dev_get_drvdata(dev); struct nouveau_drm *drm = nouveau_drm(drm_dev); struct nvkm_volt *volt = nvxx_volt(&drm->client.device); - int ret; + int ret = 0; if (!volt) return -EOPNOTSUPP; @@ -499,7 +504,8 @@ nouveau_in_read(struct device *dev, u32 attr, int channel, long *val) switch (attr) { case hwmon_in_input: ret = nvkm_volt_get(volt); - *val = ret < 0 ? ret : (ret / 1000); + if (ret >= 0) + *val = ret / 1000; break; case hwmon_in_min: *val = volt->min_uv > 0 ? (volt->min_uv / 1000) : -ENODEV; @@ -511,7 +517,7 @@ nouveau_in_read(struct device *dev, u32 attr, int channel, long *val) return -EOPNOTSUPP; } - return 0; + return ret; } static int @@ -520,21 +526,26 @@ nouveau_pwm_read(struct device *dev, u32 attr, int channel, long *val) struct drm_device *drm_dev = dev_get_drvdata(dev); struct nouveau_drm *drm = nouveau_drm(drm_dev); struct nvkm_therm *therm = nvxx_therm(&drm->client.device); + int ret; if (!therm || !therm->attr_get || !therm->fan_get) return -EOPNOTSUPP; switch (attr) { case hwmon_pwm_enable: - *val = therm->attr_get(therm, NVKM_THERM_ATTR_FAN_MODE); + ret = therm->attr_get(therm, NVKM_THERM_ATTR_FAN_MODE); break; case hwmon_pwm_input: - *val = therm->fan_get(therm); + ret = therm->fan_get(therm); break; default: return -EOPNOTSUPP; } + if (ret < 0) + return ret; + + *val = ret; return 0; } @@ -544,18 +555,26 @@ nouveau_power_read(struct device *dev, u32 attr, int channel, long *val) struct drm_device *drm_dev = dev_get_drvdata(dev); struct nouveau_drm *drm = nouveau_drm(drm_dev); struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device); + int ret; if (!iccsense) return -EOPNOTSUPP; switch (attr) { case hwmon_power_input: - *val = nvkm_iccsense_read_all(iccsense); + ret = nvkm_iccsense_read_all(iccsense); + if (ret < 0) + return ret; + *val = ret; break; case hwmon_power_max: + if (iccsense->power_w_max <= 0) + return -ENODEV; *val = iccsense->power_w_max; break; case hwmon_power_crit: + if (iccsense->power_w_crit <= 0) + return -ENODEV; *val = iccsense->power_w_crit; break; default: -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 03/29] subdev/volt/gk104: return error when read fails
While my gpu was powered off, hwmon returned 0.6V as the current voltage. If nvkm_rd32 fails for any reason, return the error. With that sensors will display a "N/A" instead of 0.6V. Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nvkm/subdev/volt/gk104.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drm/nouveau/nvkm/subdev/volt/gk104.c b/drm/nouveau/nvkm/subdev/volt/gk104.c index 1c744e02..53a7af9d 100644 --- a/drm/nouveau/nvkm/subdev/volt/gk104.c +++ b/drm/nouveau/nvkm/subdev/volt/gk104.c @@ -40,10 +40,15 @@ gk104_volt_get(struct nvkm_volt *base) { struct nvbios_volt *bios = &gk104_volt(base)->bios; struct nvkm_device *device = base->subdev.device; - u32 div, duty; + int div, duty; div = nvkm_rd32(device, 0x20340); + if (div < 0) + return div; + duty = nvkm_rd32(device, 0x20344); + if (duty < 0) + return duty; return bios->base + bios->pwm_range * duty / div; } -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 04/29] 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 v2: remove parameter name Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/subdev/clk.h | 1 + drm/nouveau/nvkm/subdev/clk/base.c | 26 ++++++++++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h index e5275f74..ce3bbcfe 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 *, 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.14.1
We won't need it now, because we will adjust the clocks depending on engine loads later on anyway or a static lookup table. It also simplifies the clocking logic. This code was nowhere used anyway and just a mock up. v2: fixed typo in commit message Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/include/nvkm/subdev/clk.h | 2 -- drm/nouveau/nvkm/subdev/clk/base.c | 16 ++-------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h index ce3bbcfe..1340f5b8 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 *, 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.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 06/29] clk: Make pstate a pointer to nvkm_pstate
We will access the current cstate at least every second and this saves us some CPU cycles looking them up every second. v2: Rewording commit message. Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/include/nvkm/subdev/clk.h | 4 +++- drm/nouveau/nvkm/engine/device/ctrl.c | 5 ++++- drm/nouveau/nvkm/subdev/clk/base.c | 17 ++++++++++++----- drm/nouveau/nvkm/subdev/pmu/gk20a.c | 18 +++++++----------- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h index 1340f5b8..ec537e08 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 05e81855..de579726 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.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 07/29] clk: Hold information about the current cstate status
Later we will have situations where the expected and the current state isn't the same. Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/include/nvkm/subdev/clk.h | 2 ++ drm/nouveau/nvkm/subdev/clk/base.c | 32 +++++++++++++++++++++++++------- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h index ec537e08..f35518c3 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.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 08/29] clk: We should pass the pstate id around not the index in the list
This makes the code easier, because we can compare the id with pstate->pstate and saves us from the trouble of iterating over the pstates to match the index. v2: reword commit message Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/nouveau_debugfs.c | 6 +-- drm/nouveau/nvkm/subdev/clk/base.c | 78 +++++++++++++++++++------------------- 2 files changed, 41 insertions(+), 43 deletions(-) diff --git a/drm/nouveau/nouveau_debugfs.c b/drm/nouveau/nouveau_debugfs.c index 963a4dba..27281c4e 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; } @@ -490,33 +492,10 @@ 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.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 09/29] clk: Set clocks to pre suspend state after suspend
The idea is to clear out the saved state, because after a resume we can't know what the GPU is clocked to. The reclock is triggered by the call to nvkm_clk_update later in nvkm_clk_init. v2: convert to C style comments Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/nvkm/subdev/clk/base.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c index 1d71bf09..54188d2b 100644 --- a/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drm/nouveau/nvkm/subdev/clk/base.c @@ -625,11 +625,10 @@ 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; } @@ -683,8 +682,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.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 10/29] core/device: Move therm behind clk
Later therm will depend on clk reporting new temperatures and triggereing reclocks for thermal throttling or therm related voltage/clock adjustments. Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/core/device.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drm/nouveau/include/nvkm/core/device.h b/drm/nouveau/include/nvkm/core/device.h index bb4c214f..44a99c3c 100644 --- a/drm/nouveau/include/nvkm/core/device.h +++ b/drm/nouveau/include/nvkm/core/device.h @@ -24,8 +24,8 @@ enum nvkm_devidx { NVKM_SUBDEV_PMU, NVKM_SUBDEV_VOLT, NVKM_SUBDEV_ICCSENSE, - NVKM_SUBDEV_THERM, NVKM_SUBDEV_CLK, + NVKM_SUBDEV_THERM, NVKM_SUBDEV_SECBOOT, NVKM_ENGINE_BSP, -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 11/29] debugfs: Wake up GPU before doing any reclocking
Fixes various reclocking related issues on prime systems Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nouveau_debugfs.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drm/nouveau/nouveau_debugfs.c b/drm/nouveau/nouveau_debugfs.c index 27281c4e..b0a598f9 100644 --- a/drm/nouveau/nouveau_debugfs.c +++ b/drm/nouveau/nouveau_debugfs.c @@ -160,7 +160,11 @@ nouveau_debugfs_pstate_set(struct file *file, const char __user *ubuf, args.ustate = value; } + ret = pm_runtime_get_sync(drm->dev); + if (IS_ERR_VALUE(ret) && ret != -EACCES) + return ret; ret = nvif_mthd(ctrl, NVIF_CONTROL_PSTATE_USER, &args, sizeof(args)); + pm_runtime_put_autosuspend(drm->dev); if (ret < 0) return ret; -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 12/29] therm: Don't cancel the timer
We will need a always running therm daemon to adjust the voltage/clocks on the fly. Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/nvkm/subdev/therm/base.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drm/nouveau/nvkm/subdev/therm/base.c b/drm/nouveau/nvkm/subdev/therm/base.c index 8fa691fb..4e69cd8e 100644 --- a/drm/nouveau/nvkm/subdev/therm/base.c +++ b/drm/nouveau/nvkm/subdev/therm/base.c @@ -114,7 +114,6 @@ nvkm_therm_update(struct nvkm_therm *therm, int mode) struct nvkm_timer *tmr = subdev->device->timer; unsigned long flags; bool immd = true; - bool poll = true; int duty = -1; spin_lock_irqsave(&therm->lock, flags); @@ -124,11 +123,9 @@ nvkm_therm_update(struct nvkm_therm *therm, int mode) switch (mode) { case NVKM_THERM_CTRL_MANUAL: - nvkm_timer_alarm(tmr, 0, &therm->alarm); duty = nvkm_therm_fan_get(therm); if (duty < 0) duty = 100; - poll = false; break; case NVKM_THERM_CTRL_AUTO: switch(therm->fan->bios.fan_mode) { @@ -143,18 +140,16 @@ nvkm_therm_update(struct nvkm_therm *therm, int mode) duty = therm->cstate; else duty = nvkm_therm_update_linear_fallback(therm); - poll = false; break; } immd = false; break; case NVKM_THERM_CTRL_NONE: default: - nvkm_timer_alarm(tmr, 0, &therm->alarm); - poll = false; + break; } - if (poll) + if (list_empty(&therm->alarm.head)) nvkm_timer_alarm(tmr, 1000000000ULL, &therm->alarm); spin_unlock_irqrestore(&therm->lock, flags); -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 13/29] therm: Move the temp readout into the alarm
It makes more sense to read out the temperature in the alarm, because we want to do various things with it: 1. adjust the fans 2. notify the clk subdev about the changed temperature Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/subdev/therm.h | 1 + drm/nouveau/nvkm/subdev/therm/base.c | 43 +++++++++++++++------------------ 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/drm/nouveau/include/nvkm/subdev/therm.h b/drm/nouveau/include/nvkm/subdev/therm.h index 8c84017f..27670497 100644 --- a/drm/nouveau/include/nvkm/subdev/therm.h +++ b/drm/nouveau/include/nvkm/subdev/therm.h @@ -56,6 +56,7 @@ struct nvkm_therm { int mode; int cstate; int suspend; + u8 last_temp; /* bios */ struct nvbios_therm_sensor bios_sensor; diff --git a/drm/nouveau/nvkm/subdev/therm/base.c b/drm/nouveau/nvkm/subdev/therm/base.c index 4e69cd8e..59f01fec 100644 --- a/drm/nouveau/nvkm/subdev/therm/base.c +++ b/drm/nouveau/nvkm/subdev/therm/base.c @@ -32,18 +32,13 @@ nvkm_therm_temp_get(struct nvkm_therm *therm, int *val) } static int -nvkm_therm_update_trip(struct nvkm_therm *therm) +nvkm_therm_update_trip(struct nvkm_therm *therm, u8 temp) { - int temp, ret; struct nvbios_therm_trip_point *trip = therm->fan->bios.trip, *cur_trip = NULL, *last_trip = therm->last_trip; u16 duty, i; - ret = therm->func->temp_get(therm, &temp); - if (ret < 0) - return ret; - /* look for the trip point corresponding to the current temperature */ cur_trip = NULL; for (i = 0; i < therm->fan->bios.nr_fan_trip; i++) { @@ -69,15 +64,10 @@ nvkm_therm_update_trip(struct nvkm_therm *therm) static int nvkm_therm_compute_linear_duty(struct nvkm_therm *therm, u8 linear_min_temp, - u8 linear_max_temp) + u8 linear_max_temp, u8 temp) { - int temp, ret; u16 duty; - ret = therm->func->temp_get(therm, &temp); - if (ret < 0) - return ret; - /* handle the non-linear part first */ if (temp < linear_min_temp) return therm->fan->bios.min_duty; @@ -93,22 +83,22 @@ nvkm_therm_compute_linear_duty(struct nvkm_therm *therm, u8 linear_min_temp, } static int -nvkm_therm_update_linear(struct nvkm_therm *therm) +nvkm_therm_update_linear(struct nvkm_therm *therm, u8 temp) { u8 min = therm->fan->bios.linear_min_temp; u8 max = therm->fan->bios.linear_max_temp; - return nvkm_therm_compute_linear_duty(therm, min, max); + return nvkm_therm_compute_linear_duty(therm, min, max, temp); } static int -nvkm_therm_update_linear_fallback(struct nvkm_therm *therm) +nvkm_therm_update_linear_fallback(struct nvkm_therm *therm, u8 temp) { u8 max = therm->bios_sensor.thrs_fan_boost.temp; - return nvkm_therm_compute_linear_duty(therm, 30, max); + return nvkm_therm_compute_linear_duty(therm, 30, max, temp); } static void -nvkm_therm_update(struct nvkm_therm *therm, int mode) +nvkm_therm_update(struct nvkm_therm *therm, u8 temp, int mode) { struct nvkm_subdev *subdev = &therm->subdev; struct nvkm_timer *tmr = subdev->device->timer; @@ -130,16 +120,16 @@ nvkm_therm_update(struct nvkm_therm *therm, int mode) case NVKM_THERM_CTRL_AUTO: switch(therm->fan->bios.fan_mode) { case NVBIOS_THERM_FAN_TRIP: - duty = nvkm_therm_update_trip(therm); + duty = nvkm_therm_update_trip(therm, temp); break; case NVBIOS_THERM_FAN_LINEAR: - duty = nvkm_therm_update_linear(therm); + duty = nvkm_therm_update_linear(therm, temp); break; case NVBIOS_THERM_FAN_OTHER: if (therm->cstate) duty = therm->cstate; else - duty = nvkm_therm_update_linear_fallback(therm); + duty = nvkm_therm_update_linear_fallback(therm, temp); break; } immd = false; @@ -167,7 +157,7 @@ nvkm_therm_cstate(struct nvkm_therm *therm, int fan, int dir) (dir > 0 && fan > therm->cstate)) { nvkm_debug(subdev, "default fan speed -> %d%%\n", fan); therm->cstate = fan; - nvkm_therm_update(therm, -1); + nvkm_therm_update(therm, therm->last_temp, -1); } return 0; } @@ -175,9 +165,12 @@ nvkm_therm_cstate(struct nvkm_therm *therm, int fan, int dir) static void nvkm_therm_alarm(struct nvkm_alarm *alarm) { + int temp; struct nvkm_therm *therm container_of(alarm, struct nvkm_therm, alarm); - nvkm_therm_update(therm, -1); + if (nvkm_therm_temp_get(therm, &temp) >= 0) + therm->last_temp = temp; + nvkm_therm_update(therm, therm->last_temp, -1); } int @@ -208,7 +201,7 @@ nvkm_therm_fan_mode(struct nvkm_therm *therm, int mode) return 0; nvkm_debug(subdev, "fan management: %s\n", name[mode]); - nvkm_therm_update(therm, mode); + nvkm_therm_update(therm, therm->last_temp, mode); return 0; } @@ -343,11 +336,15 @@ nvkm_therm_oneinit(struct nvkm_subdev *subdev) static int nvkm_therm_init(struct nvkm_subdev *subdev) { + int temp; struct nvkm_therm *therm = nvkm_therm(subdev); if (therm->func->init) therm->func->init(therm); + nvkm_therm_temp_get(therm, &temp); + therm->last_temp = temp; + if (therm->suspend >= 0) { /* restore the pwm value only when on manual or auto mode */ if (therm->suspend > 0) -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 14/29] therm: Trigger reclock in temperature daemon
Depending on the temperature, cstates might become unreachable or the maped voltage of a cstate changes. We want to adjust to that. Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/nvkm/subdev/therm/base.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drm/nouveau/nvkm/subdev/therm/base.c b/drm/nouveau/nvkm/subdev/therm/base.c index 59f01fec..c90b9cfc 100644 --- a/drm/nouveau/nvkm/subdev/therm/base.c +++ b/drm/nouveau/nvkm/subdev/therm/base.c @@ -23,6 +23,8 @@ */ #include "priv.h" +#include <subdev/clk.h> + int nvkm_therm_temp_get(struct nvkm_therm *therm, int *val) { @@ -168,9 +170,15 @@ nvkm_therm_alarm(struct nvkm_alarm *alarm) int temp; struct nvkm_therm *therm container_of(alarm, struct nvkm_therm, alarm); + struct nvkm_clk *clk = therm->subdev.device->clk; + if (nvkm_therm_temp_get(therm, &temp) >= 0) therm->last_temp = temp; + nvkm_therm_update(therm, therm->last_temp, -1); + + if (clk) + nvkm_clk_tstate(clk, therm->last_temp); } int -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 15/29] bios: add thermal policies table
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- .../include/nvkm/subdev/bios/thermal_policies.h | 27 ++++++++ drm/nouveau/nvkm/subdev/bios/Kbuild | 1 + drm/nouveau/nvkm/subdev/bios/thermal_policies.c | 81 ++++++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h create mode 100644 drm/nouveau/nvkm/subdev/bios/thermal_policies.c diff --git a/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h b/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h new file mode 100644 index 00000000..c9215fdd --- /dev/null +++ b/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h @@ -0,0 +1,27 @@ +#ifndef __NVBIOS_THERMAL_POLICIES_H__ +#define __NVBIOS_THERMAL_POLICIES_H__ + +struct nvbios_thermal_policies_header { + u32 offset; + + u8 version; + u8 hlen; + u8 ecount; + u8 elen; +}; +struct nvbios_thermal_policies_entry { + u8 mode; + u16 t0; + u16 t1; + u16 t2; + s16 down_offset; + s16 up_offset; +}; + +int nvbios_thermal_policies_parse(struct nvkm_bios *, + struct nvbios_thermal_policies_header *); +int nvbios_thermal_policies_entry(struct nvkm_bios *, + struct nvbios_thermal_policies_header *, + u8 idx, + struct nvbios_thermal_policies_entry *); +#endif diff --git a/drm/nouveau/nvkm/subdev/bios/Kbuild b/drm/nouveau/nvkm/subdev/bios/Kbuild index 6b4f1e06..38f31dd0 100644 --- a/drm/nouveau/nvkm/subdev/bios/Kbuild +++ b/drm/nouveau/nvkm/subdev/bios/Kbuild @@ -30,6 +30,7 @@ nvkm-y += nvkm/subdev/bios/shadowramin.o nvkm-y += nvkm/subdev/bios/shadowrom.o nvkm-y += nvkm/subdev/bios/timing.o nvkm-y += nvkm/subdev/bios/therm.o +nvkm-y += nvkm/subdev/bios/thermal_policies.o nvkm-y += nvkm/subdev/bios/vmap.o nvkm-y += nvkm/subdev/bios/volt.o nvkm-y += nvkm/subdev/bios/vpstate.o diff --git a/drm/nouveau/nvkm/subdev/bios/thermal_policies.c b/drm/nouveau/nvkm/subdev/bios/thermal_policies.c new file mode 100644 index 00000000..5105194e --- /dev/null +++ b/drm/nouveau/nvkm/subdev/bios/thermal_policies.c @@ -0,0 +1,81 @@ +/* + * Copyright 2017 Karol Herbst + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * Authors: Karol Herbst + */ +#include <subdev/bios.h> +#include <subdev/bios/bit.h> +#include <subdev/bios/thermal_policies.h> + +static u32 +nvbios_thermal_policies_offset(struct nvkm_bios *b) +{ + struct bit_entry bit_P; + + if (!bit_entry(b, 'P', &bit_P)) { + if (bit_P.version == 2 && bit_P.length >= 0x50) + return nvbios_rd32(b, bit_P.offset + 0x50); + } + + return 0; +} + +int +nvbios_thermal_policies_parse(struct nvkm_bios *b, struct nvbios_thermal_policies_header *h) +{ + if (!h) + return -EINVAL; + + h->offset = nvbios_thermal_policies_offset(b); + if (!h->offset) + return -ENODEV; + + h->version = nvbios_rd08(b, h->offset); + switch (h->version) { + case 0x10: + h->hlen = nvbios_rd08(b, h->offset + 0x1); + h->elen = nvbios_rd08(b, h->offset + 0x2); + h->ecount = nvbios_rd08(b, h->offset + 0x3); + return 0; + default: + return -EINVAL; + } +} + +int +nvbios_thermal_policies_entry(struct nvkm_bios *b, struct nvbios_thermal_policies_header *h, + u8 idx, struct nvbios_thermal_policies_entry *e) +{ + u32 offset; + + if (!e || !h || idx > h->ecount) + return -EINVAL; + + offset = h->offset + h->hlen + idx * h->elen; + e->mode = nvbios_rd08(b, offset); + e->t0 = nvbios_rd16(b, offset + 0x2); + e->t1 = nvbios_rd16(b, offset + 0x4); + e->t2 = nvbios_rd16(b, offset + 0x6); + e->down_offset = nvbios_rd16(b, offset + 0x12); + e->up_offset = nvbios_rd16(b, offset + 0x14); + + return 0; +} -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 16/29] clk: parse thermal policies for throttling thresholds
v2: use min_t Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/subdev/clk.h | 2 ++ drm/nouveau/nvkm/subdev/clk/base.c | 42 +++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h index f35518c3..f5ff1fd9 100644 --- a/drm/nouveau/include/nvkm/subdev/clk.h +++ b/drm/nouveau/include/nvkm/subdev/clk.h @@ -104,6 +104,8 @@ struct nvkm_clk { struct nvkm_cstate *cstate; int exp_cstateid; u8 temp; + u8 max_temp; + u8 relax_temp; bool allow_reclock; #define NVKM_CLK_BOOST_NONE 0x0 diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c index 54188d2b..54e14936 100644 --- a/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drm/nouveau/nvkm/subdev/clk/base.c @@ -27,6 +27,7 @@ #include <subdev/bios/boost.h> #include <subdev/bios/cstep.h> #include <subdev/bios/perf.h> +#include <subdev/bios/thermal_policies.h> #include <subdev/bios/vpstate.h> #include <subdev/fb.h> #include <subdev/therm.h> @@ -659,6 +660,44 @@ nvkm_clk = { .fini = nvkm_clk_fini, }; +static void +nvkm_clk_parse_max_temp(struct nvkm_clk *clk) +{ + struct nvkm_subdev *subdev = &clk->subdev; + struct nvkm_bios *bios = subdev->device->bios; + struct nvbios_thermal_policies_header header; + struct nvbios_thermal_policies_entry entry; + u8 i; + s16 mt = 0xff; + s16 rt = 0xff; + + if (nvbios_thermal_policies_parse(bios, &header)) + return; + + if (!header.ecount) + return; + + for (i = 0; i < header.ecount; i++) { + if (nvbios_thermal_policies_entry(bios, &header, i, &entry)) + return; + + if (entry.mode != 1) + continue; + + mt = min_t(s16, mt, (entry.t0 + entry.down_offset) / 32); + rt = min_t(s16, rt, (entry.t0 + entry.up_offset) / 32); + } + + if (mt == 0xff || rt == 0xff) + return; + + clk->max_temp = mt; + clk->relax_temp = rt; + + nvkm_debug(subdev, "setting up sw throttling thresholds (%u/%u°C)\n", + clk->max_temp, clk->relax_temp); +} + int nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, int index, bool allow_reclock, struct nvkm_clk *clk) @@ -733,6 +772,9 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, clk->boost_mode = nvkm_longopt(device->cfgopt, "NvBoost", NVKM_CLK_BOOST_NONE); + + nvkm_clk_parse_max_temp(clk); + return 0; } -- 2.14.1
v2: make message about relaxed throttling an info rework reporting about current clk state Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/subdev/clk.h | 1 + drm/nouveau/nvkm/subdev/clk/base.c | 44 +++++++++++++++++++++++++---------- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h index f5ff1fd9..b79bf657 100644 --- a/drm/nouveau/include/nvkm/subdev/clk.h +++ b/drm/nouveau/include/nvkm/subdev/clk.h @@ -106,6 +106,7 @@ struct nvkm_clk { u8 temp; u8 max_temp; u8 relax_temp; + bool throttled; bool allow_reclock; #define NVKM_CLK_BOOST_NONE 0x0 diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c index 54e14936..72ca5e0c 100644 --- a/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drm/nouveau/nvkm/subdev/clk/base.c @@ -279,7 +279,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid) struct nvkm_fb *fb = subdev->device->fb; struct nvkm_pci *pci = subdev->device->pci; struct nvkm_pstate *pstate; - int ret; + int ret, cstate; if (pstateid == NVKM_CLK_PSTATE_DEFAULT) return 0; @@ -308,7 +308,12 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid) ram->func->tidy(ram); } - return nvkm_cstate_prog(clk, pstate, clk->exp_cstateid); + if (clk->throttled) + cstate = list_first_entry(&pstate->list, struct nvkm_cstate, head)->id; + else + cstate = clk->exp_cstateid; + + return nvkm_cstate_prog(clk, pstate, cstate); } static void @@ -322,22 +327,20 @@ 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 C %d T %d°C\n", - pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc, - clk->astate, clk->exp_cstateid, clk->temp); - pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc; if (clk->state_nr && pstate != -1) { - pstate = (pstate < 0) ? clk->astate : pstate; + if (clk->throttled) + pstate = list_first_entry(&clk->states, struct nvkm_pstate, head)->pstate; + else + pstate = (pstate < 0) ? clk->astate : pstate; } else { pstate = NVKM_CLK_PSTATE_DEFAULT; } - nvkm_trace(subdev, "-> %d\n", pstate); + nvkm_trace(subdev, "PWR %d U(AC) %d U(DC) %d A %d C %d T %d°C\n", + clk->pwrsrc, clk->ustate_ac, clk->ustate_dc, + clk->astate, clk->exp_cstateid, clk->temp); + if (!clk->pstate || pstate != clk->pstate->pstate) { int ret = nvkm_pstate_prog(clk, pstate); if (ret) { @@ -564,9 +567,25 @@ nvkm_clk_astate(struct nvkm_clk *clk, int req, int rel, bool wait) int nvkm_clk_tstate(struct nvkm_clk *clk, u8 temp) { + struct nvkm_subdev *subdev = &clk->subdev; if (clk->temp == temp) return 0; clk->temp = temp; + if (clk->max_temp && clk->relax_temp) { + if (!clk->throttled && temp > clk->max_temp) { + nvkm_warn(subdev, + "temperature (%d C) hit the 'downclock' " + "threshold\n", + temp); + clk->throttled = true; + } else if (clk->throttled && temp < clk->relax_temp) { + nvkm_info(subdev, + "temperature (%d C) went below the " + "'relax' threshold\n", + temp); + clk->throttled = false; + } + } return nvkm_clk_update(clk, false); } @@ -727,6 +746,7 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, clk->ustate_dc = -1; clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT; clk->temp = 90; /* reasonable default value */ + clk->throttled = false; clk->allow_reclock = allow_reclock; -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 18/29] clk: Only do partial reclocks as required
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nvkm/subdev/clk/base.c | 26 ++++++++++++++++++-------- drm/nouveau/nvkm/subdev/volt/base.c | 3 +++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c index 72ca5e0c..a154a425 100644 --- a/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drm/nouveau/nvkm/subdev/clk/base.c @@ -171,7 +171,7 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei) struct nvkm_therm *therm = device->therm; struct nvkm_volt *volt = device->volt; struct nvkm_cstate *cstate; - int ret; + int ret = 0; if (cstatei == NVKM_CLK_CSTATE_DEFAULT) return 0; @@ -183,6 +183,12 @@ nvkm_cstate_prog(struct nvkm_clk *clk, struct nvkm_pstate *pstate, int cstatei) cstate = &pstate->base; } + // if the cstate matches, just update the voltage + if (clk->cstate && clk->cstate == cstate) + return nvkm_volt_set_id(volt, clk->cstate->voltage, + clk->pstate->base.voltage, clk->temp, + 0); + if (therm) { ret = nvkm_therm_cstate(therm, pstate->fanspeed, +1); if (ret && ret != -ENODEV) { @@ -284,6 +290,11 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid) if (pstateid == NVKM_CLK_PSTATE_DEFAULT) return 0; + if (clk->pstate && clk->pstate->pstate == pstateid) { + pstate = clk->pstate; + goto cstate; + } + list_for_each_entry(pstate, &clk->states, head) { if (pstate->pstate == pstateid) break; @@ -308,6 +319,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid) ram->func->tidy(ram); } +cstate: if (clk->throttled) cstate = list_first_entry(&pstate->list, struct nvkm_cstate, head)->id; else @@ -321,7 +333,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; - int pstate; + int pstate, ret; if (!atomic_xchg(&clk->waiting, 0)) return; @@ -341,12 +353,10 @@ nvkm_clk_update_work(struct work_struct *work) clk->pwrsrc, clk->ustate_ac, clk->ustate_dc, clk->astate, clk->exp_cstateid, clk->temp); - 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); - } + ret = nvkm_pstate_prog(clk, pstate); + if (ret) { + nvkm_error(subdev, "error setting pstate %d: %d\n", + pstate, ret); } wake_up_all(&clk->wait); diff --git a/drm/nouveau/nvkm/subdev/volt/base.c b/drm/nouveau/nvkm/subdev/volt/base.c index e344901c..56a0f379 100644 --- a/drm/nouveau/nvkm/subdev/volt/base.c +++ b/drm/nouveau/nvkm/subdev/volt/base.c @@ -162,6 +162,9 @@ nvkm_volt_set_id(struct nvkm_volt *volt, u8 id, u8 min_id, u8 temp, { int ret; + if (!volt) + return -ENODEV; + if (volt->func->set_id) return volt->func->set_id(volt, id, condition); -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 19/29] secboot/acr352: reset PMU after secboot
This is needed for using Nouveaus PMU image after performing secboot. This will be helpfull for Maxwell2 reclocking on boards without externally controlled fans like on most laptops or fanless boards. Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nvkm/subdev/secboot/acr_r352.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drm/nouveau/nvkm/subdev/secboot/acr_r352.c b/drm/nouveau/nvkm/subdev/secboot/acr_r352.c index a7213542..00095ef8 100644 --- a/drm/nouveau/nvkm/subdev/secboot/acr_r352.c +++ b/drm/nouveau/nvkm/subdev/secboot/acr_r352.c @@ -924,6 +924,19 @@ acr_r352_bootstrap(struct acr_r352 *acr, struct nvkm_secboot *sb) } } + /* reset the PMU if needed */ + if (acr->base.boot_falcon == NVKM_SECBOOT_FALCON_PMU && + !nvkm_secboot_is_managed(sb, NVKM_SECBOOT_FALCON_PMU)) { + struct nvkm_pmu *pmu = subdev->device->pmu; + if (pmu) { + ret = nvkm_subdev_init(&pmu->subdev); + if (ret < 0) { + nvkm_error(subdev, "Failed to reset PMU\n"); + return ret; + } + } + } + return 0; } -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 20/29] device: enable clk for Maxwell2
Reclokcing will only be enabled by setting NvFanless to true. Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nvkm/engine/device/base.c | 3 +++ drm/nouveau/nvkm/subdev/clk/gk104.c | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drm/nouveau/nvkm/engine/device/base.c b/drm/nouveau/nvkm/engine/device/base.c index 28fd4fa9..c1ede28a 100644 --- a/drm/nouveau/nvkm/engine/device/base.c +++ b/drm/nouveau/nvkm/engine/device/base.c @@ -2029,6 +2029,7 @@ nv120_chipset = { .bar = gf100_bar_new, .bios = nvkm_bios_new, .bus = gf100_bus_new, + .clk = gk104_clk_new, .devinit = gm200_devinit_new, .fb = gm200_fb_new, .fuse = gm107_fuse_new, @@ -2064,6 +2065,7 @@ nv124_chipset = { .bar = gf100_bar_new, .bios = nvkm_bios_new, .bus = gf100_bus_new, + .clk = gk104_clk_new, .devinit = gm200_devinit_new, .fb = gm200_fb_new, .fuse = gm107_fuse_new, @@ -2099,6 +2101,7 @@ nv126_chipset = { .bar = gf100_bar_new, .bios = nvkm_bios_new, .bus = gf100_bus_new, + .clk = gk104_clk_new, .devinit = gm200_devinit_new, .fb = gm200_fb_new, .fuse = gm107_fuse_new, diff --git a/drm/nouveau/nvkm/subdev/clk/gk104.c b/drm/nouveau/nvkm/subdev/clk/gk104.c index 0b37e3da..be04463e 100644 --- a/drm/nouveau/nvkm/subdev/clk/gk104.c +++ b/drm/nouveau/nvkm/subdev/clk/gk104.c @@ -25,6 +25,7 @@ #include "priv.h" #include "pll.h" +#include <core/option.h> #include <subdev/timer.h> #include <subdev/bios.h> #include <subdev/bios/pll.h> @@ -507,10 +508,17 @@ int gk104_clk_new(struct nvkm_device *device, int index, struct nvkm_clk **pclk) { struct gk104_clk *clk; + bool reclocking; if (!(clk = kzalloc(sizeof(*clk), GFP_KERNEL))) return -ENOMEM; *pclk = &clk->base; - return nvkm_clk_ctor(&gk104_clk, device, index, true, &clk->base); + if (device->chipset >= 0x120) + reclocking = nvkm_boolopt(device->cfgopt, "NvFanless", false); + else + reclocking = true; + + return nvkm_clk_ctor(&gk104_clk, device, index, reclocking, + &clk->base); } -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 21/29] clk: Save the max clock we can set
Saving the highest possible clock from the vpstate domain makes it easier to read it out whenever we want. Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/include/nvkm/subdev/clk.h | 1 + drm/nouveau/nvkm/subdev/clk/base.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h index b79bf657..4c225dcb 100644 --- a/drm/nouveau/include/nvkm/subdev/clk.h +++ b/drm/nouveau/include/nvkm/subdev/clk.h @@ -115,6 +115,7 @@ struct nvkm_clk { u8 boost_mode; u32 base_khz; u32 boost_khz; + u32 max_khz; /*XXX: die, these are here *only* to support the completely * bat-shit insane what-was-nouveau_hw.c code diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c index a154a425..0fd92790 100644 --- a/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drm/nouveau/nvkm/subdev/clk/base.c @@ -267,6 +267,8 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct nvkm_pstate *pstate) u32 freq = nvkm_clk_adjust(clk, true, pstate->pstate, domain->bios, cstepX.freq); cstate->domain[domain->name] = freq; + if (domain->flags & NVKM_CLK_DOM_FLAG_VPSTATE) + clk->max_khz = max(clk->max_khz, freq); } domain++; } -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 22/29] nvif: Add boost info and set operations
v5: Return ENODEV on devices without any vpstates. Fail earlier if not supported. Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/include/nvif/if0001.h | 15 +++++++++ drm/nouveau/nvkm/engine/device/ctrl.c | 61 +++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/drm/nouveau/include/nvif/if0001.h b/drm/nouveau/include/nvif/if0001.h index bd5b6412..e4acd195 100644 --- a/drm/nouveau/include/nvif/if0001.h +++ b/drm/nouveau/include/nvif/if0001.h @@ -4,6 +4,8 @@ #define NVIF_CONTROL_PSTATE_INFO 0x00 #define NVIF_CONTROL_PSTATE_ATTR 0x01 #define NVIF_CONTROL_PSTATE_USER 0x02 +#define NVIF_CONTROL_BOOST_INFO 0x03 +#define NVIF_CONTROL_BOOST_SET 0x04 struct nvif_control_pstate_info_v0 { __u8 version; @@ -43,4 +45,17 @@ struct nvif_control_pstate_user_v0 { __s8 pwrsrc; /* in: target power source */ __u8 pad03[5]; }; + +struct nvif_control_boost_info_v0 { + __u8 version; + __u8 mode; + __u16 base_mhz; + __u16 boost_mhz; + __u16 max_mhz; +}; + +struct nvif_control_boost_set_v0 { + __u8 version; + __u8 mode; +}; #endif diff --git a/drm/nouveau/nvkm/engine/device/ctrl.c b/drm/nouveau/nvkm/engine/device/ctrl.c index da70626c..0c44e4d2 100644 --- a/drm/nouveau/nvkm/engine/device/ctrl.c +++ b/drm/nouveau/nvkm/engine/device/ctrl.c @@ -169,6 +169,63 @@ nvkm_control_mthd_pstate_user(struct nvkm_control *ctrl, void *data, u32 size) return ret; } +static int +nvkm_control_mthd_boost_info(struct nvkm_control *ctrl, void *data, u32 size) +{ + union { + struct nvif_control_boost_info_v0 v0; + } *args = data; + struct nvkm_clk *clk = ctrl->device->clk; + int ret = -ENOSYS; + + if (!clk) + return -ENODEV; + + if (!clk->base_khz && !clk->boost_khz) + return -ENODEV; + + nvif_ioctl(&ctrl->object, "control boost info size %d\n", size); + if (!(ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, false))) { + nvif_ioctl(&ctrl->object, "control boost info vers %d\n", + args->v0.version); + } else + return ret; + + args->v0.mode = clk->boost_mode; + args->v0.base_mhz = clk->base_khz / 2000; + args->v0.boost_mhz = clk->boost_khz / 2000; + args->v0.max_mhz = clk->max_khz / 2000; + return 0; +} + +static int +nvkm_control_mthd_boost_set(struct nvkm_control *ctrl, void *data, u32 size) +{ + union { + struct nvif_control_boost_set_v0 v0; + } *args = data; + struct nvkm_clk *clk = ctrl->device->clk; + int ret = -ENOSYS; + + if (!clk) + return -ENODEV; + + if (!clk->base_khz && !clk->boost_khz) + return -ENODEV; + + nvif_ioctl(&ctrl->object, "control boost set size %d\n", size); + if (!(ret = nvif_unpack(ret, &data, &size, args->v0, 0, 0, false))) { + nvif_ioctl(&ctrl->object, "control boost set vers %d\n", + args->v0.version); + } else + return ret; + + if (args->v0.mode > 2) + return -EINVAL; + clk->boost_mode = args->v0.mode; + return nvkm_clk_update(clk, true); +} + static int nvkm_control_mthd(struct nvkm_object *object, u32 mthd, void *data, u32 size) { @@ -180,6 +237,10 @@ nvkm_control_mthd(struct nvkm_object *object, u32 mthd, void *data, u32 size) return nvkm_control_mthd_pstate_attr(ctrl, data, size); case NVIF_CONTROL_PSTATE_USER: return nvkm_control_mthd_pstate_user(ctrl, data, size); + case NVIF_CONTROL_BOOST_INFO: + return nvkm_control_mthd_boost_info(ctrl, data, size); + case NVIF_CONTROL_BOOST_SET: + return nvkm_control_mthd_boost_set(ctrl, data, size); default: break; } -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 23/29] debugfs: Add boost interface to change the boost_mode
Signed-off-by: Karol Herbst <karolherbst at gmail.com> Reviewed-by: Martin Peres <martin.peres at free.fr> --- drm/nouveau/nouveau_debugfs.c | 80 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/drm/nouveau/nouveau_debugfs.c b/drm/nouveau/nouveau_debugfs.c index b0a598f9..c19cc535 100644 --- a/drm/nouveau/nouveau_debugfs.c +++ b/drm/nouveau/nouveau_debugfs.c @@ -184,6 +184,85 @@ static const struct file_operations nouveau_pstate_fops = { .write = nouveau_debugfs_pstate_set, }; +static void +nouveau_debugfs_boost_get_entry(struct seq_file *m, u8 mode, u8 entry, u16 value) +{ + if (value) { + if (mode == entry) + seq_printf(m, "*%i", entry); + else + seq_printf(m, " %i", entry); + seq_printf(m, ": %u MHz\n", value); + } +} + +static int +nouveau_debugfs_boost_get(struct seq_file *m, void *data) +{ + struct drm_device *drm = m->private; + struct nouveau_debugfs *debugfs = nouveau_debugfs(drm); + struct nvif_object *ctrl = &debugfs->ctrl; + struct nvif_control_boost_info_v0 info = {}; + int ret; + + ret = nvif_mthd(ctrl, NVIF_CONTROL_BOOST_INFO, &info, sizeof(info)); + if (ret) + return ret; + + nouveau_debugfs_boost_get_entry(m, info.mode, 0, info.base_mhz); + nouveau_debugfs_boost_get_entry(m, info.mode, 1, info.boost_mhz); + nouveau_debugfs_boost_get_entry(m, info.mode, 2, info.max_mhz); + return 0; +} + +static ssize_t +nouveau_debugfs_boost_set(struct file *file, const char __user *ubuf, + size_t len, loff_t *offp) +{ + struct seq_file *m = file->private_data; + struct drm_device *drm = m->private; + struct nouveau_debugfs *debugfs = nouveau_debugfs(drm); + struct nvif_object *ctrl = &debugfs->ctrl; + struct nvif_control_boost_set_v0 args = {}; + char buf[3] = {}; + long ret; + u8 value; + + if (len >= sizeof(buf)) + return -EINVAL; + + if (copy_from_user(buf, ubuf, len)) + return -EFAULT; + + ret = kstrtou8(buf, 10, &value); + if (ret) + return ret; + + args.mode = value; + ret = pm_runtime_get_sync(drm->dev); + if (IS_ERR_VALUE(ret) && ret != -EACCES) + return ret; + ret = nvif_mthd(ctrl, NVIF_CONTROL_BOOST_SET, &args, sizeof(args)); + pm_runtime_put_autosuspend(drm->dev); + if (ret) + return ret; + + return len; +} + +static int +nouveau_debugfs_boost_open(struct inode *inode, struct file *file) +{ + return single_open(file, nouveau_debugfs_boost_get, inode->i_private); +} + +static const struct file_operations nouveau_boost_fops = { + .owner = THIS_MODULE, + .open = nouveau_debugfs_boost_open, + .read = seq_read, + .write = nouveau_debugfs_boost_set, +}; + static struct drm_info_list nouveau_debugfs_list[] = { { "vbios.rom", nouveau_debugfs_vbios_image, 0, NULL }, }; @@ -193,6 +272,7 @@ static const struct nouveau_debugfs_files { const char *name; const struct file_operations *fops; } nouveau_debugfs_files[] = { + {"boost", &nouveau_boost_fops}, {"pstate", &nouveau_pstate_fops}, }; -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 24/29] bios/vpstate: there are some fermi vbios with no boost or tdp entry
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nvkm/subdev/bios/vpstate.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drm/nouveau/nvkm/subdev/bios/vpstate.c b/drm/nouveau/nvkm/subdev/bios/vpstate.c index 20b6fc82..71524548 100644 --- a/drm/nouveau/nvkm/subdev/bios/vpstate.c +++ b/drm/nouveau/nvkm/subdev/bios/vpstate.c @@ -58,8 +58,14 @@ nvbios_vpstate_parse(struct nvkm_bios *b, struct nvbios_vpstate_header *h) h->ecount = nvbios_rd08(b, h->offset + 0x5); h->base_id = nvbios_rd08(b, h->offset + 0x0f); - h->boost_id = nvbios_rd08(b, h->offset + 0x10); - h->tdp_id = nvbios_rd08(b, h->offset + 0x11); + if (h->hlen > 0x10) + h->boost_id = nvbios_rd08(b, h->offset + 0x10); + else + h->boost_id = 0xff; + if (h->hlen > 0x11) + h->tdp_id = nvbios_rd08(b, h->offset + 0x11); + else + h->tdp_id = 0xff; return 0; default: return -EINVAL; -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 25/29] bios/vpstate: parse max battery id
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/subdev/bios/vpstate.h | 1 + drm/nouveau/nvkm/subdev/bios/vpstate.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drm/nouveau/include/nvkm/subdev/bios/vpstate.h b/drm/nouveau/include/nvkm/subdev/bios/vpstate.h index 87f804fc..181de47b 100644 --- a/drm/nouveau/include/nvkm/subdev/bios/vpstate.h +++ b/drm/nouveau/include/nvkm/subdev/bios/vpstate.h @@ -10,6 +10,7 @@ struct nvbios_vpstate_header { u8 scount; u8 slen; + u8 battery_id; u8 base_id; u8 boost_id; u8 tdp_id; diff --git a/drm/nouveau/nvkm/subdev/bios/vpstate.c b/drm/nouveau/nvkm/subdev/bios/vpstate.c index 71524548..c1de6421 100644 --- a/drm/nouveau/nvkm/subdev/bios/vpstate.c +++ b/drm/nouveau/nvkm/subdev/bios/vpstate.c @@ -57,7 +57,8 @@ nvbios_vpstate_parse(struct nvkm_bios *b, struct nvbios_vpstate_header *h) h->scount = nvbios_rd08(b, h->offset + 0x4); h->ecount = nvbios_rd08(b, h->offset + 0x5); - h->base_id = nvbios_rd08(b, h->offset + 0x0f); + h->battery_id = nvbios_rd08(b, h->offset + 0x0c); + h->base_id = nvbios_rd08(b, h->offset + 0x0f); if (h->hlen > 0x10) h->boost_id = nvbios_rd08(b, h->offset + 0x10); else -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 26/29] clk: refactor the base and boost clock limits so that we can limit pstates as well
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/subdev/clk.h | 9 +++++++-- drm/nouveau/nvkm/engine/device/ctrl.c | 8 ++++---- drm/nouveau/nvkm/subdev/clk/base.c | 34 +++++++++++++++++++++++++--------- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h index 4c225dcb..70505c9b 100644 --- a/drm/nouveau/include/nvkm/subdev/clk.h +++ b/drm/nouveau/include/nvkm/subdev/clk.h @@ -81,6 +81,11 @@ struct nvkm_domain { int mdiv; }; +struct nvkm_clk_limit { + u8 pstate; + u32 max_khz; +}; + struct nvkm_clk { const struct nvkm_clk_func *func; struct nvkm_subdev subdev; @@ -113,8 +118,8 @@ struct nvkm_clk { #define NVKM_CLK_BOOST_BIOS 0x1 #define NVKM_CLK_BOOST_FULL 0x2 u8 boost_mode; - u32 base_khz; - u32 boost_khz; + struct nvkm_clk_limit base_limit; + struct nvkm_clk_limit boost_limit; u32 max_khz; /*XXX: die, these are here *only* to support the completely diff --git a/drm/nouveau/nvkm/engine/device/ctrl.c b/drm/nouveau/nvkm/engine/device/ctrl.c index 0c44e4d2..d521cd05 100644 --- a/drm/nouveau/nvkm/engine/device/ctrl.c +++ b/drm/nouveau/nvkm/engine/device/ctrl.c @@ -181,7 +181,7 @@ nvkm_control_mthd_boost_info(struct nvkm_control *ctrl, void *data, u32 size) if (!clk) return -ENODEV; - if (!clk->base_khz && !clk->boost_khz) + if (!clk->base_limit.max_khz && !clk->boost_limit.max_khz) return -ENODEV; nvif_ioctl(&ctrl->object, "control boost info size %d\n", size); @@ -192,8 +192,8 @@ nvkm_control_mthd_boost_info(struct nvkm_control *ctrl, void *data, u32 size) return ret; args->v0.mode = clk->boost_mode; - args->v0.base_mhz = clk->base_khz / 2000; - args->v0.boost_mhz = clk->boost_khz / 2000; + args->v0.base_mhz = clk->base_limit.max_khz / 2000; + args->v0.boost_mhz = clk->boost_limit.max_khz / 2000; args->v0.max_mhz = clk->max_khz / 2000; return 0; } @@ -210,7 +210,7 @@ nvkm_control_mthd_boost_set(struct nvkm_control *ctrl, void *data, u32 size) if (!clk) return -ENODEV; - if (!clk->base_khz && !clk->boost_khz) + if (!clk->base_limit.max_khz && !clk->boost_limit.max_khz) return -ENODEV; nvif_ioctl(&ctrl->object, "control boost set size %d\n", size); diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c index 0fd92790..9f44ace0 100644 --- a/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drm/nouveau/nvkm/subdev/clk/base.c @@ -87,14 +87,22 @@ nvkm_cstate_valid(struct nvkm_clk *clk, struct nvkm_cstate *cstate, while (domain && domain->name != nv_clk_src_max) { if (domain->flags & NVKM_CLK_DOM_FLAG_VPSTATE) { u32 freq = cstate->domain[domain->name]; + u32 limit; switch (clk->boost_mode) { case NVKM_CLK_BOOST_NONE: - if (clk->base_khz && freq > clk->base_khz) - return false; + limit = clk->base_limit.max_khz; + if (limit) + break; case NVKM_CLK_BOOST_BIOS: - if (clk->boost_khz && freq > clk->boost_khz) - return false; + limit = clk->boost_limit.max_khz; + break; + default: + limit = 0; + break; } + + if (limit && freq > limit) + return false; } domain++; } @@ -729,6 +737,14 @@ nvkm_clk_parse_max_temp(struct nvkm_clk *clk) clk->max_temp, clk->relax_temp); } +void static +nvkm_clk_fill_limit(struct nvkm_clk_limit *l, + const struct nvbios_vpstate_entry *e) +{ + l->pstate = e->pstate; + l->max_khz = e->clock_mhz * 1000; +} + int nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, int index, bool allow_reclock, struct nvkm_clk *clk) @@ -742,11 +758,11 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, nvkm_subdev_ctor(&nvkm_clk, device, index, subdev); if (bios && !nvbios_vpstate_parse(bios, &h)) { - struct nvbios_vpstate_entry base, boost; - if (!nvbios_vpstate_entry(bios, &h, h.boost_id, &boost)) - clk->boost_khz = boost.clock_mhz * 1000; - if (!nvbios_vpstate_entry(bios, &h, h.base_id, &base)) - clk->base_khz = base.clock_mhz * 1000; + struct nvbios_vpstate_entry vpe; + if (!nvbios_vpstate_entry(bios, &h, h.boost_id, &vpe)) + nvkm_clk_fill_limit(&clk->boost_limit, &vpe); + if (!nvbios_vpstate_entry(bios, &h, h.base_id, &vpe)) + nvkm_clk_fill_limit(&clk->base_limit, &vpe); } clk->func = func; -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 27/29] clk: implement limiting pstates just like we do for cstates
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nvkm/subdev/clk/base.c | 62 ++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c index 9f44ace0..2a51c078 100644 --- a/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drm/nouveau/nvkm/subdev/clk/base.c @@ -288,6 +288,52 @@ nvkm_cstate_new(struct nvkm_clk *clk, int idx, struct nvkm_pstate *pstate) /****************************************************************************** * P-States *****************************************************************************/ +static struct nvkm_pstate * +nvkm_pstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *start) +{ + struct nvkm_pstate *pstate; + u8 limit; + + switch (clk->boost_mode) { + case NVKM_CLK_BOOST_NONE: + limit = clk->base_limit.pstate; + if (limit) + break; + case NVKM_CLK_BOOST_BIOS: + limit = clk->boost_limit.pstate; + break; + default: + limit = 0; + break; + } + + if (!limit) + return start; + + for (pstate = start; &pstate->head != &clk->states; + pstate = list_entry(pstate->head.prev, typeof(*pstate), head)) { + if (limit >= pstate->pstate) + break; + } + return pstate; +} + +static struct nvkm_pstate * +nvkm_pstate_get(struct nvkm_clk *clk, int pstateid) +{ + struct nvkm_pstate *pstate; + switch (pstateid) { + case NVKM_CLK_PSTATE_DEFAULT: + return NULL; + default: + list_for_each_entry(pstate, &clk->states, head) { + if (pstate->pstate == pstateid) + return pstate; + } + } + return NULL; +} + static int nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid) { @@ -300,20 +346,16 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid) if (pstateid == NVKM_CLK_PSTATE_DEFAULT) return 0; - if (clk->pstate && clk->pstate->pstate == pstateid) { - pstate = clk->pstate; - goto cstate; - } - - list_for_each_entry(pstate, &clk->states, head) { - if (pstate->pstate == pstateid) - break; - } + pstate = nvkm_pstate_get(clk, pstateid); + pstate = nvkm_pstate_find_best(clk, pstate); if (!pstate) return -EINVAL; - nvkm_debug(subdev, "setting performance state %x\n", pstateid); + if (clk->pstate && clk->pstate == pstate) + goto cstate; + + nvkm_debug(subdev, "setting performance state %x\n", pstate->pstate); clk->pstate = pstate; nvkm_pcie_set_link(pci, pstate->pcie_speed, pstate->pcie_width); -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 28/29] clk: move the switch out of the loop in nvkm_cstate_valid
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nvkm/subdev/clk/base.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c index 2a51c078..994d32b9 100644 --- a/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drm/nouveau/nvkm/subdev/clk/base.c @@ -83,28 +83,30 @@ nvkm_cstate_valid(struct nvkm_clk *clk, struct nvkm_cstate *cstate, const struct nvkm_domain *domain = clk->domains; struct nvkm_volt *volt = clk->subdev.device->volt; int voltage; + u32 limit; - while (domain && domain->name != nv_clk_src_max) { - if (domain->flags & NVKM_CLK_DOM_FLAG_VPSTATE) { - u32 freq = cstate->domain[domain->name]; - u32 limit; - switch (clk->boost_mode) { - case NVKM_CLK_BOOST_NONE: - limit = clk->base_limit.max_khz; - if (limit) - break; - case NVKM_CLK_BOOST_BIOS: - limit = clk->boost_limit.max_khz; - break; - default: - limit = 0; - break; - } + switch (clk->boost_mode) { + case NVKM_CLK_BOOST_NONE: + limit = clk->base_limit.max_khz; + if (limit) + break; + case NVKM_CLK_BOOST_BIOS: + limit = clk->boost_limit.max_khz; + break; + default: + limit = 0; + break; + } + if (limit) { + for (; domain && domain->name != nv_clk_src_max; domain++) { + u32 freq; + if (!(domain->flags & NVKM_CLK_DOM_FLAG_VPSTATE)) + continue; + freq = cstate->domain[domain->name]; if (limit && freq > limit) return false; } - domain++; } if (!volt) -- 2.14.1
Karol Herbst
2017-Sep-15 17:11 UTC
[Nouveau] [RFC PATCH 29/29] clk: limit clocks on battery
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/subdev/clk.h | 1 + drm/nouveau/nvkm/subdev/clk/base.c | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h index 70505c9b..fae45e6e 100644 --- a/drm/nouveau/include/nvkm/subdev/clk.h +++ b/drm/nouveau/include/nvkm/subdev/clk.h @@ -120,6 +120,7 @@ struct nvkm_clk { u8 boost_mode; struct nvkm_clk_limit base_limit; struct nvkm_clk_limit boost_limit; + struct nvkm_clk_limit batt_limit; u32 max_khz; /*XXX: die, these are here *only* to support the completely diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c index 994d32b9..0de5b3dc 100644 --- a/drm/nouveau/nvkm/subdev/clk/base.c +++ b/drm/nouveau/nvkm/subdev/clk/base.c @@ -98,6 +98,14 @@ nvkm_cstate_valid(struct nvkm_clk *clk, struct nvkm_cstate *cstate, break; } + if (!clk->pwrsrc) { + u32 blimit = clk->batt_limit.max_khz; + if (!limit) + limit = blimit; + else if (blimit) + limit = min(blimit, limit); + } + if (limit) { for (; domain && domain->name != nv_clk_src_max; domain++) { u32 freq; @@ -309,6 +317,14 @@ nvkm_pstate_find_best(struct nvkm_clk *clk, struct nvkm_pstate *start) break; } + if (!clk->pwrsrc) { + u8 blimit = clk->batt_limit.pstate; + if (!limit) + limit = blimit; + else if (blimit) + limit = min(blimit, limit); + } + if (!limit) return start; @@ -807,6 +823,8 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, nvkm_clk_fill_limit(&clk->boost_limit, &vpe); if (!nvbios_vpstate_entry(bios, &h, h.base_id, &vpe)) nvkm_clk_fill_limit(&clk->base_limit, &vpe); + if (!nvbios_vpstate_entry(bios, &h, h.battery_id, &vpe)) + nvkm_clk_fill_limit(&clk->batt_limit, &vpe); } clk->func = func; -- 2.14.1
Pierre Moreau
2017-Oct-08 10:16 UTC
[Nouveau] [RFC PATCH 01/29] therm: split return code and value in nvkm_get_temp
As you changed the return value of `temp_get()` to solely be the error code, or absence of an error, I would change all those tests that checked whether the returned value was strictly less, or greater than, 0 to now only compare against 0 (no error). For example, if (therm && therm->attr_get && nvkm_therm_temp_get(therm, &val) < 0) if (therm->func->temp_get(therm, &val) >= 0) could become if (therm && therm->attr_get && nvkm_therm_temp_get(therm, &val)) if (!therm->func->temp_get(therm, &val)) to match other error checking code. More comments below On 2017-09-15 — 17:11, Karol Herbst wrote:> The current hwmon code doesn't check if the returned value was actually an > error. > > Since Kepler temperature sensors are able to report negative values.I assume those negative values are not for error reporting, but more if you buried your GPU in the snow somewhere in the Antarctic and still want a valid temperature to be reported.> Since Pascal (and maybe earlier) we have sensors with improved precision. > > Adjust the nvkm_get_temp method to be able to deal with those changes > and let hwmon return an error properly. > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > drm/nouveau/include/nvkm/subdev/therm.h | 2 +- > drm/nouveau/nouveau_hwmon.c | 15 +++++++++------ > drm/nouveau/nvkm/subdev/therm/base.c | 19 ++++++++++++++----- > drm/nouveau/nvkm/subdev/therm/g84.c | 11 ++++++----- > drm/nouveau/nvkm/subdev/therm/gp100.c | 9 +++++---- > drm/nouveau/nvkm/subdev/therm/nv40.c | 9 +++------ > drm/nouveau/nvkm/subdev/therm/nv50.c | 9 +++------ > drm/nouveau/nvkm/subdev/therm/priv.h | 4 ++-- > drm/nouveau/nvkm/subdev/therm/temp.c | 16 ++++++++++++---- > 9 files changed, 55 insertions(+), 39 deletions(-) > > diff --git a/drm/nouveau/include/nvkm/subdev/therm.h b/drm/nouveau/include/nvkm/subdev/therm.h > index 9841f076..8c84017f 100644 > --- a/drm/nouveau/include/nvkm/subdev/therm.h > +++ b/drm/nouveau/include/nvkm/subdev/therm.h > @@ -86,7 +86,7 @@ struct nvkm_therm { > int (*attr_set)(struct nvkm_therm *, enum nvkm_therm_attr_type, int); > }; > > -int nvkm_therm_temp_get(struct nvkm_therm *); > +int nvkm_therm_temp_get(struct nvkm_therm *, int *); > int nvkm_therm_fan_sense(struct nvkm_therm *); > int nvkm_therm_cstate(struct nvkm_therm *, int, int); > > diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c > index 7c965648..f1914d9a 100644 > --- a/drm/nouveau/nouveau_hwmon.c > +++ b/drm/nouveau/nouveau_hwmon.c > @@ -326,8 +326,9 @@ nouveau_temp_is_visible(const void *data, u32 attr, int channel) > { > struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data); > struct nvkm_therm *therm = nvxx_therm(&drm->client.device); > + int val; > > - if (therm && therm->attr_get && nvkm_therm_temp_get(therm) < 0) > + if (therm && therm->attr_get && nvkm_therm_temp_get(therm, &val) < 0) > return 0; > > switch (attr) { > @@ -421,15 +422,16 @@ nouveau_temp_read(struct device *dev, u32 attr, int channel, long *val) > struct drm_device *drm_dev = dev_get_drvdata(dev); > struct nouveau_drm *drm = nouveau_drm(drm_dev); > struct nvkm_therm *therm = nvxx_therm(&drm->client.device); > - int ret; > + int ret = 0; > + int temp; > > if (!therm || !therm->attr_get) > return -EOPNOTSUPP; > > switch (attr) { > case hwmon_temp_input: > - ret = nvkm_therm_temp_get(therm); > - *val = ret < 0 ? ret : (ret * 1000); > + ret = nvkm_therm_temp_get(therm, &temp); > + *val = temp * 1000; > break; > case hwmon_temp_max: > *val = therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK) > @@ -459,7 +461,7 @@ nouveau_temp_read(struct device *dev, u32 attr, int channel, long *val) > return -EOPNOTSUPP; > } > > - return 0; > + return ret; > } > > static int > @@ -713,6 +715,7 @@ nouveau_hwmon_init(struct drm_device *dev) > struct device *hwmon_dev; > int ret = 0; > int i = 0; > + int val; > > hwmon = drm->hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL); > if (!hwmon) > @@ -720,7 +723,7 @@ nouveau_hwmon_init(struct drm_device *dev) > hwmon->dev = dev; > > if (therm && therm->attr_get && therm->attr_set) { > - if (nvkm_therm_temp_get(therm) >= 0) > + if (nvkm_therm_temp_get(therm, &val) >= 0) > special_groups[i++] = &temp1_auto_point_sensor_group; > if (therm->fan_get && therm->fan_get(therm) >= 0) > special_groups[i++] = &pwm_fan_sensor_group; > diff --git a/drm/nouveau/nvkm/subdev/therm/base.c b/drm/nouveau/nvkm/subdev/therm/base.c > index f27fc6d0..8fa691fb 100644 > --- a/drm/nouveau/nvkm/subdev/therm/base.c > +++ b/drm/nouveau/nvkm/subdev/therm/base.c > @@ -24,22 +24,26 @@ > #include "priv.h" > > int > -nvkm_therm_temp_get(struct nvkm_therm *therm) > +nvkm_therm_temp_get(struct nvkm_therm *therm, int *val) > { > if (therm->func->temp_get) > - return therm->func->temp_get(therm); > + return therm->func->temp_get(therm, val); > return -ENODEV; > } > > static int > nvkm_therm_update_trip(struct nvkm_therm *therm) > { > + int temp, ret; > struct nvbios_therm_trip_point *trip = therm->fan->bios.trip, > *cur_trip = NULL, > *last_trip = therm->last_trip; > - u8 temp = therm->func->temp_get(therm); > u16 duty, i; > > + ret = therm->func->temp_get(therm, &temp); > + if (ret < 0) > + return ret; > + > /* look for the trip point corresponding to the current temperature */ > cur_trip = NULL; > for (i = 0; i < therm->fan->bios.nr_fan_trip; i++) { > @@ -67,9 +71,13 @@ static int > nvkm_therm_compute_linear_duty(struct nvkm_therm *therm, u8 linear_min_temp, > u8 linear_max_temp) > { > - u8 temp = therm->func->temp_get(therm); > + int temp, ret; > u16 duty; > > + ret = therm->func->temp_get(therm, &temp); > + if (ret < 0) > + return ret; > + > /* handle the non-linear part first */ > if (temp < linear_min_temp) > return therm->fan->bios.min_duty; > @@ -182,6 +190,7 @@ nvkm_therm_fan_mode(struct nvkm_therm *therm, int mode) > { > struct nvkm_subdev *subdev = &therm->subdev; > struct nvkm_device *device = subdev->device; > + int val; > static const char *name[] = { > "disabled", > "manual", > @@ -197,7 +206,7 @@ nvkm_therm_fan_mode(struct nvkm_therm *therm, int mode) > /* do not allow automatic fan management if the thermal sensor is > * not available */ > if (mode == NVKM_THERM_CTRL_AUTO && > - therm->func->temp_get(therm) < 0) > + therm->func->temp_get(therm, &val) < 0) > return -EINVAL; > > if (therm->mode == mode) > diff --git a/drm/nouveau/nvkm/subdev/therm/g84.c b/drm/nouveau/nvkm/subdev/therm/g84.c > index 96f8da40..3bb2d829 100644 > --- a/drm/nouveau/nvkm/subdev/therm/g84.c > +++ b/drm/nouveau/nvkm/subdev/therm/g84.c > @@ -27,14 +27,15 @@ > #include <subdev/fuse.h> > > int > -g84_temp_get(struct nvkm_therm *therm) > +g84_temp_get(struct nvkm_therm *therm, int *val) > { > struct nvkm_device *device = therm->subdev.device; > > - if (nvkm_fuse_read(device->fuse, 0x1a8) == 1) > - return nvkm_rd32(device, 0x20400); > - else > + if (nvkm_fuse_read(device->fuse, 0x1a8) != 1) > return -ENODEV; > + > + *val = nvkm_rd32(device, 0x20400);You could allow passing NULL values for `val` for when you are not interested by the results, as you did in some places further up. I would probably check for NULL values for `val`, and for `therm` though it seems to be an implicit rule that the caller should be the one doing the check.> + return 0; > } > > void > @@ -115,7 +116,7 @@ g84_therm_threshold_hyst_emulation(struct nvkm_therm *therm, > } > > /* fix the state (in case someone reprogrammed the alarms) */ > - cur = therm->func->temp_get(therm); > + therm->func->temp_get(therm, &cur);Should there be a check for the return value of the `temp_get()` function here, and error out if an error code was returned? In any case, you are the behaviour here, as if `temp_get()` previously returned an error code, the if-statement would always be false, whereas now, it’s undecided as `cur` will not even be initialised in that case! I am not sure whether it should be the caller of `temp_get()` or `temp_get()` itself which should initialise a default value for the `val` argument (probably the latter).> if (new_state == NVKM_THERM_THRS_LOWER && cur > thrs->temp) > new_state = NVKM_THERM_THRS_HIGHER; > else if (new_state == NVKM_THERM_THRS_HIGHER && > diff --git a/drm/nouveau/nvkm/subdev/therm/gp100.c b/drm/nouveau/nvkm/subdev/therm/gp100.c > index 9f0dea3f..d8206748 100644 > --- a/drm/nouveau/nvkm/subdev/therm/gp100.c > +++ b/drm/nouveau/nvkm/subdev/therm/gp100.c > @@ -24,7 +24,7 @@ > #include "priv.h" > > static int > -gp100_temp_get(struct nvkm_therm *therm) > +gp100_temp_get(struct nvkm_therm *therm, int *val) > { > struct nvkm_device *device = therm->subdev.device; > struct nvkm_subdev *subdev = &therm->subdev; > @@ -36,9 +36,10 @@ gp100_temp_get(struct nvkm_therm *therm) > nvkm_trace(subdev, "reading temperature from SHADOWed sensor\n"); > > /* device valid */ > - if (tsensor & 0x20000000) > - return (inttemp >> 8); > - else > + if (tsensor & 0x20000000) { > + *val = inttemp >> 8; > + return 0; > + } else > return -ENODEV; > } > > diff --git a/drm/nouveau/nvkm/subdev/therm/nv40.c b/drm/nouveau/nvkm/subdev/therm/nv40.c > index 2c92ffb5..cfd5b215 100644 > --- a/drm/nouveau/nvkm/subdev/therm/nv40.c > +++ b/drm/nouveau/nvkm/subdev/therm/nv40.c > @@ -70,7 +70,7 @@ nv40_sensor_setup(struct nvkm_therm *therm) > } > > static int > -nv40_temp_get(struct nvkm_therm *therm) > +nv40_temp_get(struct nvkm_therm *therm, int *val) > { > struct nvkm_device *device = therm->subdev.device; > struct nvbios_therm_sensor *sensor = &therm->bios_sensor; > @@ -95,11 +95,8 @@ nv40_temp_get(struct nvkm_therm *therm) > core_temp = core_temp + sensor->offset_num / sensor->offset_den; > core_temp = core_temp + sensor->offset_constant - 8; > > - /* reserve negative temperatures for errors */ > - if (core_temp < 0) > - core_temp = 0; > - > - return core_temp; > + *val = core_temp; > + return 0; > } > > static int > diff --git a/drm/nouveau/nvkm/subdev/therm/nv50.c b/drm/nouveau/nvkm/subdev/therm/nv50.c > index 9b57b433..62ec4063 100644 > --- a/drm/nouveau/nvkm/subdev/therm/nv50.c > +++ b/drm/nouveau/nvkm/subdev/therm/nv50.c > @@ -126,7 +126,7 @@ nv50_sensor_setup(struct nvkm_therm *therm) > } > > static int > -nv50_temp_get(struct nvkm_therm *therm) > +nv50_temp_get(struct nvkm_therm *therm, int *val) > { > struct nvkm_device *device = therm->subdev.device; > struct nvbios_therm_sensor *sensor = &therm->bios_sensor; > @@ -143,11 +143,8 @@ nv50_temp_get(struct nvkm_therm *therm) > core_temp = core_temp + sensor->offset_num / sensor->offset_den; > core_temp = core_temp + sensor->offset_constant - 8; > > - /* reserve negative temperatures for errors */ > - if (core_temp < 0) > - core_temp = 0; > - > - return core_temp; > + *val = core_temp; > + return 0; > } > > static void > diff --git a/drm/nouveau/nvkm/subdev/therm/priv.h b/drm/nouveau/nvkm/subdev/therm/priv.h > index 1f46e371..b325ec5f 100644 > --- a/drm/nouveau/nvkm/subdev/therm/priv.h > +++ b/drm/nouveau/nvkm/subdev/therm/priv.h > @@ -91,7 +91,7 @@ struct nvkm_therm_func { > int (*pwm_set)(struct nvkm_therm *, int line, u32, u32); > int (*pwm_clock)(struct nvkm_therm *, int line); > > - int (*temp_get)(struct nvkm_therm *); > + int (*temp_get)(struct nvkm_therm *, int *); > > int (*fan_sense)(struct nvkm_therm *); > > @@ -105,7 +105,7 @@ int nv50_fan_pwm_get(struct nvkm_therm *, int, u32 *, u32 *); > int nv50_fan_pwm_set(struct nvkm_therm *, int, u32, u32); > int nv50_fan_pwm_clock(struct nvkm_therm *, int); > > -int g84_temp_get(struct nvkm_therm *); > +int g84_temp_get(struct nvkm_therm *, int *); > void g84_sensor_setup(struct nvkm_therm *); > void g84_therm_fini(struct nvkm_therm *); > > diff --git a/drm/nouveau/nvkm/subdev/therm/temp.c b/drm/nouveau/nvkm/subdev/therm/temp.c > index ddb2b2c6..5ec2dfb3 100644 > --- a/drm/nouveau/nvkm/subdev/therm/temp.c > +++ b/drm/nouveau/nvkm/subdev/therm/temp.c > @@ -86,7 +86,10 @@ nvkm_therm_sensor_event(struct nvkm_therm *therm, enum nvkm_therm_thrs thrs, > static const char * const thresholds[] = { > "fanboost", "downclock", "critical", "shutdown" > }; > - int temperature = therm->func->temp_get(therm); > + int temperature; > + > + if (therm->func->temp_get(therm, &temperature) < 0) > + return; > > if (thrs < 0 || thrs > 3) > return; > @@ -140,7 +143,10 @@ nvkm_therm_threshold_hyst_polling(struct nvkm_therm *therm, > { > enum nvkm_therm_thrs_direction direction; > enum nvkm_therm_thrs_state prev_state, new_state; > - int temp = therm->func->temp_get(therm); > + int temp; > + > + if (therm->func->temp_get(therm, &temp) < 0) > + return; > > prev_state = nvkm_therm_sensor_get_threshold_state(therm, thrs_name); > > @@ -166,6 +172,7 @@ alarm_timer_callback(struct nvkm_alarm *alarm) > struct nvbios_therm_sensor *sensor = &therm->bios_sensor; > struct nvkm_timer *tmr = therm->subdev.device->timer; > unsigned long flags; > + int val; > > spin_lock_irqsave(&therm->sensor.alarm_program_lock, flags); > > @@ -185,7 +192,7 @@ alarm_timer_callback(struct nvkm_alarm *alarm) > spin_unlock_irqrestore(&therm->sensor.alarm_program_lock, flags); > > /* schedule the next poll in one second */ > - if (therm->func->temp_get(therm) >= 0) > + if (therm->func->temp_get(therm, &val) >= 0) > nvkm_timer_alarm(tmr, 1000000000ULL, alarm); > } > > @@ -227,9 +234,10 @@ nvkm_therm_sensor_fini(struct nvkm_therm *therm, bool suspend) > void > nvkm_therm_sensor_preinit(struct nvkm_therm *therm) > { > + int val; > const char *sensor_avail = "yes"; > > - if (therm->func->temp_get(therm) < 0) > + if (therm->func->temp_get(therm, &val) < 0) > sensor_avail = "no"; > > nvkm_debug(&therm->subdev, "internal sensor: %s\n", sensor_avail); > -- > 2.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/050cfaac/attachment.sig>
Pierre Moreau
2017-Oct-08 10:22 UTC
[Nouveau] [RFC PATCH 02/29] hwmon: properly check for errors
Reviewed-by: Pierre Moreau <pierre.morrow at free.fr> On 2017-09-15 — 17:11, Karol Herbst wrote:> Otherwise hwmon interprets error codes as real values. > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > drm/nouveau/nouveau_hwmon.c | 33 ++++++++++++++++++++++++++------- > 1 file changed, 26 insertions(+), 7 deletions(-) > > diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c > index f1914d9a..0d75c14d 100644 > --- a/drm/nouveau/nouveau_hwmon.c > +++ b/drm/nouveau/nouveau_hwmon.c > @@ -470,18 +470,23 @@ nouveau_fan_read(struct device *dev, u32 attr, int channel, long *val) > struct drm_device *drm_dev = dev_get_drvdata(dev); > struct nouveau_drm *drm = nouveau_drm(drm_dev); > struct nvkm_therm *therm = nvxx_therm(&drm->client.device); > + int ret; > > if (!therm) > return -EOPNOTSUPP; > > switch (attr) { > case hwmon_fan_input: > - *val = nvkm_therm_fan_sense(therm); > + ret = nvkm_therm_fan_sense(therm); > break; > default: > return -EOPNOTSUPP; > } > > + if (ret < 0) > + return ret; > + > + *val = ret; > return 0; > } > > @@ -491,7 +496,7 @@ nouveau_in_read(struct device *dev, u32 attr, int channel, long *val) > struct drm_device *drm_dev = dev_get_drvdata(dev); > struct nouveau_drm *drm = nouveau_drm(drm_dev); > struct nvkm_volt *volt = nvxx_volt(&drm->client.device); > - int ret; > + int ret = 0; > > if (!volt) > return -EOPNOTSUPP; > @@ -499,7 +504,8 @@ nouveau_in_read(struct device *dev, u32 attr, int channel, long *val) > switch (attr) { > case hwmon_in_input: > ret = nvkm_volt_get(volt); > - *val = ret < 0 ? ret : (ret / 1000); > + if (ret >= 0) > + *val = ret / 1000; > break; > case hwmon_in_min: > *val = volt->min_uv > 0 ? (volt->min_uv / 1000) : -ENODEV; > @@ -511,7 +517,7 @@ nouveau_in_read(struct device *dev, u32 attr, int channel, long *val) > return -EOPNOTSUPP; > } > > - return 0; > + return ret; > } > > static int > @@ -520,21 +526,26 @@ nouveau_pwm_read(struct device *dev, u32 attr, int channel, long *val) > struct drm_device *drm_dev = dev_get_drvdata(dev); > struct nouveau_drm *drm = nouveau_drm(drm_dev); > struct nvkm_therm *therm = nvxx_therm(&drm->client.device); > + int ret; > > if (!therm || !therm->attr_get || !therm->fan_get) > return -EOPNOTSUPP; > > switch (attr) { > case hwmon_pwm_enable: > - *val = therm->attr_get(therm, NVKM_THERM_ATTR_FAN_MODE); > + ret = therm->attr_get(therm, NVKM_THERM_ATTR_FAN_MODE); > break; > case hwmon_pwm_input: > - *val = therm->fan_get(therm); > + ret = therm->fan_get(therm); > break; > default: > return -EOPNOTSUPP; > } > > + if (ret < 0) > + return ret; > + > + *val = ret; > return 0; > } > > @@ -544,18 +555,26 @@ nouveau_power_read(struct device *dev, u32 attr, int channel, long *val) > struct drm_device *drm_dev = dev_get_drvdata(dev); > struct nouveau_drm *drm = nouveau_drm(drm_dev); > struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->client.device); > + int ret; > > if (!iccsense) > return -EOPNOTSUPP; > > switch (attr) { > case hwmon_power_input: > - *val = nvkm_iccsense_read_all(iccsense); > + ret = nvkm_iccsense_read_all(iccsense); > + if (ret < 0) > + return ret; > + *val = ret; > break; > case hwmon_power_max: > + if (iccsense->power_w_max <= 0) > + return -ENODEV; > *val = iccsense->power_w_max; > break; > case hwmon_power_crit: > + if (iccsense->power_w_crit <= 0) > + return -ENODEV; > *val = iccsense->power_w_crit; > break; > default: > -- > 2.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/4913c3c7/attachment.sig>
Pierre Moreau
2017-Oct-08 10:29 UTC
[Nouveau] [RFC PATCH 03/29] subdev/volt/gk104: return error when read fails
On 2017-09-15 — 17:11, Karol Herbst wrote:> While my gpu was powered off, hwmon returned 0.6V as the current voltage. > If nvkm_rd32 fails for any reason, return the error. > > With that sensors will display a "N/A" instead of 0.6V.Small nitpick, add a comma between “that” and “sensors”. Otherwise, Reviewed-by: Pierre Moreau <pierre.morrow at free.fr>> > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > drm/nouveau/nvkm/subdev/volt/gk104.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drm/nouveau/nvkm/subdev/volt/gk104.c b/drm/nouveau/nvkm/subdev/volt/gk104.c > index 1c744e02..53a7af9d 100644 > --- a/drm/nouveau/nvkm/subdev/volt/gk104.c > +++ b/drm/nouveau/nvkm/subdev/volt/gk104.c > @@ -40,10 +40,15 @@ gk104_volt_get(struct nvkm_volt *base) > { > struct nvbios_volt *bios = &gk104_volt(base)->bios; > struct nvkm_device *device = base->subdev.device; > - u32 div, duty; > + int div, duty; > > div = nvkm_rd32(device, 0x20340); > + if (div < 0) > + return div; > + > duty = nvkm_rd32(device, 0x20344); > + if (duty < 0) > + return duty; > > return bios->base + bios->pwm_range * duty / div; > } > -- > 2.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/461f05a5/attachment-0001.sig>
Pierre Moreau
2017-Oct-08 10:41 UTC
[Nouveau] [RFC PATCH 04/29] clk: Rename nvkm_pstate_calc to nvkm_clk_update and export it
On 2017-09-15 — 17:11, 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 > > v2: remove parameter name > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > drm/nouveau/include/nvkm/subdev/clk.h | 1 + > drm/nouveau/nvkm/subdev/clk/base.c | 26 ++++++++++++++++---------- > 2 files changed, 17 insertions(+), 10 deletions(-) > > diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h > index e5275f74..ce3bbcfe 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 *, 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;Same here as below, shouldn’t there be some error checking on what `nvkm_clk_update()` returned, as it can fail?> } > > @@ -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;`nvkm_pstate_calc()` did not return any error code, but `nvkm_clk_update()` now can, so shouldn’t the function return the return value of `nvkm_clk_update()` instead? Or at least do some error checking on what `nvkm_clk_update()` returned?> } > > @@ -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.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/b22f82ac/attachment.sig>
This patch could be applied before patch 04, that way patch 04 does not need to update a function (`nvkm_clk_dstate()`) that will be removed in the next patch. Reviewed-by: Pierre Moreau <pierre.morrow at free.fr> On 2017-09-15 — 17:11, Karol Herbst wrote:> We won't need it now, because we will adjust the clocks depending on engine > loads later on anyway or a static lookup table. It also simplifies the > clocking logic. > > This code was nowhere used anyway and just a mock up. > > v2: fixed typo in commit message > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > Reviewed-by: Martin Peres <martin.peres at free.fr> > --- > drm/nouveau/include/nvkm/subdev/clk.h | 2 -- > drm/nouveau/nvkm/subdev/clk/base.c | 16 ++-------------- > 2 files changed, 2 insertions(+), 16 deletions(-) > > diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h > index ce3bbcfe..1340f5b8 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 *, 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.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/fa555405/attachment.sig>
Pierre Moreau
2017-Oct-08 11:47 UTC
[Nouveau] [RFC PATCH 06/29] clk: Make pstate a pointer to nvkm_pstate
The patch seems fine but I found it super confusing that sometimes `pstate` is a pointer (for example `clk->pstate`), sometimes it is an int (for example `args->v0.pstate`). On 2017-09-15 — 17:11, Karol Herbst wrote:> We will access the current cstate at least every second and this saves us > some CPU cycles looking them up every second. > > v2: Rewording commit message. > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > Reviewed-by: Martin Peres <martin.peres at free.fr> > --- > drm/nouveau/include/nvkm/subdev/clk.h | 4 +++- > drm/nouveau/nvkm/engine/device/ctrl.c | 5 ++++- > drm/nouveau/nvkm/subdev/clk/base.c | 17 ++++++++++++----- > drm/nouveau/nvkm/subdev/pmu/gk20a.c | 18 +++++++----------- > 4 files changed, 26 insertions(+), 18 deletions(-) > > diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h > index 1340f5b8..ec537e08 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;In the if-statement following the previous line, the -1 could be replaced by NVKM_CLK_PSTATE_DEFAULT.> @@ -315,11 +322,11 @@ nvkm_clk_update_work(struct work_struct *work) > pstate = (pstate < 0) ? clk->astate : pstate;Can `pstate` have negative values other than -1?> pstate = min(pstate, clk->state_nr - 1); > } else { > - pstate = clk->pstate = -1; > + pstate = NVKM_CLK_PSTATE_DEFAULT;Shouldn’t you also reset `clk->pstate` to NULL? Or maybe it is `nvkm_pstate_prog()` which should do it if `pstatei == 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 05e81855..de579726 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.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/9040238d/attachment.sig>
Pierre Moreau
2017-Oct-08 11:51 UTC
[Nouveau] [RFC PATCH 07/29] clk: Hold information about the current cstate status
Acked-by: Pierre Moreau <pierre.morrow at free.fr> On 2017-09-15 — 17:11, Karol Herbst wrote:> Later we will have situations where the expected and the current state > isn't the same. > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > Reviewed-by: Martin Peres <martin.peres at free.fr> > --- > drm/nouveau/include/nvkm/subdev/clk.h | 2 ++ > drm/nouveau/nvkm/subdev/clk/base.c | 32 +++++++++++++++++++++++++------- > 2 files changed, 27 insertions(+), 7 deletions(-) > > diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h > index ec537e08..f35518c3 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.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/7fa322ca/attachment.sig>
Pierre Moreau
2017-Oct-08 14:23 UTC
[Nouveau] [RFC PATCH 08/29] clk: We should pass the pstate id around not the index in the list
On 2017-09-15 — 17:11, Karol Herbst wrote:> This makes the code easier, because we can compare the id with > pstate->pstate and saves us from the trouble of iterating over the pstates > to match the index.I don’t remember whether I have already done this comment before, but I am not sure where your iterating over the pstates savings are, as in this patch at least, you iterate as much as you did before. A few more comments below> v2: reword commit message > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > Reviewed-by: Martin Peres <martin.peres at free.fr> > --- > drm/nouveau/nouveau_debugfs.c | 6 +-- > drm/nouveau/nvkm/subdev/clk/base.c | 78 +++++++++++++++++++------------------- > 2 files changed, 41 insertions(+), 43 deletions(-) > > diff --git a/drm/nouveau/nouveau_debugfs.c b/drm/nouveau/nouveau_debugfs.c > index 963a4dba..27281c4e 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; > } > @@ -490,33 +492,10 @@ 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;Don’t you need to check for `clk->allow_reclock`, as it was done in `nvkm_clk_ustate_update()`?> + > + 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;Same here, don’t you need to check for `clk->allow_reclock`, as it was done in `nvkm_clk_ustate_update()`?> + > + 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.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/198fba4e/attachment.sig>
Pierre Moreau
2017-Oct-08 14:47 UTC
[Nouveau] [RFC PATCH 09/29] clk: Set clocks to pre suspend state after suspend
Reviewed-by: Pierre Moreau <pierre.morrow at free.fr> On 2017-09-15 — 17:11, 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. > > v2: convert to C style comments > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > Reviewed-by: Martin Peres <martin.peres at free.fr> > --- > drm/nouveau/nvkm/subdev/clk/base.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c > index 1d71bf09..54188d2b 100644 > --- a/drm/nouveau/nvkm/subdev/clk/base.c > +++ b/drm/nouveau/nvkm/subdev/clk/base.c > @@ -625,11 +625,10 @@ 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; > } > @@ -683,8 +682,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.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/8412be2f/attachment.sig>
Pierre Moreau
2017-Oct-08 18:13 UTC
[Nouveau] [RFC PATCH 14/29] therm: Trigger reclock in temperature daemon
Reviewed-by: Pierre Moreau <pierre.morrow at free.fr> On 2017-09-15 — 17:11, Karol Herbst wrote:> Depending on the temperature, cstates might become unreachable or the maped > voltage of a cstate changes. We want to adjust to that. > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > Reviewed-by: Martin Peres <martin.peres at free.fr> > --- > drm/nouveau/nvkm/subdev/therm/base.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drm/nouveau/nvkm/subdev/therm/base.c b/drm/nouveau/nvkm/subdev/therm/base.c > index 59f01fec..c90b9cfc 100644 > --- a/drm/nouveau/nvkm/subdev/therm/base.c > +++ b/drm/nouveau/nvkm/subdev/therm/base.c > @@ -23,6 +23,8 @@ > */ > #include "priv.h" > > +#include <subdev/clk.h> > + > int > nvkm_therm_temp_get(struct nvkm_therm *therm, int *val) > { > @@ -168,9 +170,15 @@ nvkm_therm_alarm(struct nvkm_alarm *alarm) > int temp; > struct nvkm_therm *therm > container_of(alarm, struct nvkm_therm, alarm); > + struct nvkm_clk *clk = therm->subdev.device->clk; > + > if (nvkm_therm_temp_get(therm, &temp) >= 0) > therm->last_temp = temp; > + > nvkm_therm_update(therm, therm->last_temp, -1); > + > + if (clk) > + nvkm_clk_tstate(clk, therm->last_temp); > } > > int > -- > 2.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/121f1ee7/attachment.sig>
Pierre Moreau
2017-Oct-08 18:36 UTC
[Nouveau] [RFC PATCH 16/29] clk: parse thermal policies for throttling thresholds
On 2017-09-15 — 17:11, Karol Herbst wrote:> v2: use min_t > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > drm/nouveau/include/nvkm/subdev/clk.h | 2 ++ > drm/nouveau/nvkm/subdev/clk/base.c | 42 +++++++++++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h > index f35518c3..f5ff1fd9 100644 > --- a/drm/nouveau/include/nvkm/subdev/clk.h > +++ b/drm/nouveau/include/nvkm/subdev/clk.h > @@ -104,6 +104,8 @@ struct nvkm_clk { > struct nvkm_cstate *cstate; > int exp_cstateid; > u8 temp; > + u8 max_temp; > + u8 relax_temp;I guess those should be int, similar to the changes you made in patch 1.> > bool allow_reclock; > #define NVKM_CLK_BOOST_NONE 0x0 > diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c > index 54188d2b..54e14936 100644 > --- a/drm/nouveau/nvkm/subdev/clk/base.c > +++ b/drm/nouveau/nvkm/subdev/clk/base.c > @@ -27,6 +27,7 @@ > #include <subdev/bios/boost.h> > #include <subdev/bios/cstep.h> > #include <subdev/bios/perf.h> > +#include <subdev/bios/thermal_policies.h> > #include <subdev/bios/vpstate.h> > #include <subdev/fb.h> > #include <subdev/therm.h> > @@ -659,6 +660,44 @@ nvkm_clk = { > .fini = nvkm_clk_fini, > }; > > +static void > +nvkm_clk_parse_max_temp(struct nvkm_clk *clk) > +{ > + struct nvkm_subdev *subdev = &clk->subdev; > + struct nvkm_bios *bios = subdev->device->bios; > + struct nvbios_thermal_policies_header header; > + struct nvbios_thermal_policies_entry entry; > + u8 i; > + s16 mt = 0xff; > + s16 rt = 0xff;Why 0xff? I mean, sure, there is a high possibility that the GPU won’t like temperatures above 255℃, but still.> + > + if (nvbios_thermal_policies_parse(bios, &header)) > + return; > + > + if (!header.ecount) > + return; > + > + for (i = 0; i < header.ecount; i++) { > + if (nvbios_thermal_policies_entry(bios, &header, i, &entry)) > + return; > + > + if (entry.mode != 1) > + continue; > + > + mt = min_t(s16, mt, (entry.t0 + entry.down_offset) / 32); > + rt = min_t(s16, rt, (entry.t0 + entry.up_offset) / 32); > + } > + > + if (mt == 0xff || rt == 0xff) > + return; > + > + clk->max_temp = mt; > + clk->relax_temp = rt; > + > + nvkm_debug(subdev, "setting up sw throttling thresholds (%u/%u°C)\n", > + clk->max_temp, clk->relax_temp); > +} > + > int > nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, > int index, bool allow_reclock, struct nvkm_clk *clk) > @@ -733,6 +772,9 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, > > clk->boost_mode = nvkm_longopt(device->cfgopt, "NvBoost", > NVKM_CLK_BOOST_NONE); > + > + nvkm_clk_parse_max_temp(clk); > + > return 0; > } > > -- > 2.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/84272edb/attachment.sig>
Pierre Moreau
2017-Oct-08 18:40 UTC
[Nouveau] [RFC PATCH 15/29] bios: add thermal policies table
Looking good. It would be nice to have some defines/enums for the different modes. Some comments about t0, t1 and t2 would be nice. I saw you are using t0 in patch 16, but I have no idea why use t0 rather than t1 or t2. Otherwise, Acked-by: Pierre Moreau <pierre.morrow at free.fr> On 2017-09-15 — 17:11, Karol Herbst wrote:> Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > .../include/nvkm/subdev/bios/thermal_policies.h | 27 ++++++++ > drm/nouveau/nvkm/subdev/bios/Kbuild | 1 + > drm/nouveau/nvkm/subdev/bios/thermal_policies.c | 81 ++++++++++++++++++++++ > 3 files changed, 109 insertions(+) > create mode 100644 drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h > create mode 100644 drm/nouveau/nvkm/subdev/bios/thermal_policies.c > > diff --git a/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h b/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h > new file mode 100644 > index 00000000..c9215fdd > --- /dev/null > +++ b/drm/nouveau/include/nvkm/subdev/bios/thermal_policies.h > @@ -0,0 +1,27 @@ > +#ifndef __NVBIOS_THERMAL_POLICIES_H__ > +#define __NVBIOS_THERMAL_POLICIES_H__ > + > +struct nvbios_thermal_policies_header { > + u32 offset; > + > + u8 version; > + u8 hlen; > + u8 ecount; > + u8 elen; > +}; > +struct nvbios_thermal_policies_entry { > + u8 mode; > + u16 t0; > + u16 t1; > + u16 t2; > + s16 down_offset; > + s16 up_offset; > +}; > + > +int nvbios_thermal_policies_parse(struct nvkm_bios *, > + struct nvbios_thermal_policies_header *); > +int nvbios_thermal_policies_entry(struct nvkm_bios *, > + struct nvbios_thermal_policies_header *, > + u8 idx, > + struct nvbios_thermal_policies_entry *); > +#endif > diff --git a/drm/nouveau/nvkm/subdev/bios/Kbuild b/drm/nouveau/nvkm/subdev/bios/Kbuild > index 6b4f1e06..38f31dd0 100644 > --- a/drm/nouveau/nvkm/subdev/bios/Kbuild > +++ b/drm/nouveau/nvkm/subdev/bios/Kbuild > @@ -30,6 +30,7 @@ nvkm-y += nvkm/subdev/bios/shadowramin.o > nvkm-y += nvkm/subdev/bios/shadowrom.o > nvkm-y += nvkm/subdev/bios/timing.o > nvkm-y += nvkm/subdev/bios/therm.o > +nvkm-y += nvkm/subdev/bios/thermal_policies.o > nvkm-y += nvkm/subdev/bios/vmap.o > nvkm-y += nvkm/subdev/bios/volt.o > nvkm-y += nvkm/subdev/bios/vpstate.o > diff --git a/drm/nouveau/nvkm/subdev/bios/thermal_policies.c b/drm/nouveau/nvkm/subdev/bios/thermal_policies.c > new file mode 100644 > index 00000000..5105194e > --- /dev/null > +++ b/drm/nouveau/nvkm/subdev/bios/thermal_policies.c > @@ -0,0 +1,81 @@ > +/* > + * Copyright 2017 Karol Herbst > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > + * OTHER DEALINGS IN THE SOFTWARE. > + * > + * Authors: Karol Herbst > + */ > +#include <subdev/bios.h> > +#include <subdev/bios/bit.h> > +#include <subdev/bios/thermal_policies.h> > + > +static u32 > +nvbios_thermal_policies_offset(struct nvkm_bios *b) > +{ > + struct bit_entry bit_P; > + > + if (!bit_entry(b, 'P', &bit_P)) { > + if (bit_P.version == 2 && bit_P.length >= 0x50) > + return nvbios_rd32(b, bit_P.offset + 0x50); > + } > + > + return 0; > +} > + > +int > +nvbios_thermal_policies_parse(struct nvkm_bios *b, struct nvbios_thermal_policies_header *h) > +{ > + if (!h) > + return -EINVAL; > + > + h->offset = nvbios_thermal_policies_offset(b); > + if (!h->offset) > + return -ENODEV; > + > + h->version = nvbios_rd08(b, h->offset); > + switch (h->version) { > + case 0x10: > + h->hlen = nvbios_rd08(b, h->offset + 0x1); > + h->elen = nvbios_rd08(b, h->offset + 0x2); > + h->ecount = nvbios_rd08(b, h->offset + 0x3); > + return 0; > + default: > + return -EINVAL; > + } > +} > + > +int > +nvbios_thermal_policies_entry(struct nvkm_bios *b, struct nvbios_thermal_policies_header *h, > + u8 idx, struct nvbios_thermal_policies_entry *e) > +{ > + u32 offset; > + > + if (!e || !h || idx > h->ecount) > + return -EINVAL; > + > + offset = h->offset + h->hlen + idx * h->elen; > + e->mode = nvbios_rd08(b, offset); > + e->t0 = nvbios_rd16(b, offset + 0x2); > + e->t1 = nvbios_rd16(b, offset + 0x4); > + e->t2 = nvbios_rd16(b, offset + 0x6); > + e->down_offset = nvbios_rd16(b, offset + 0x12); > + e->up_offset = nvbios_rd16(b, offset + 0x14); > + > + return 0; > +} > -- > 2.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/52c05a44/attachment.sig>
Reviewed-by: Pierre Moreau <pierre.morrow at free.fr> On 2017-09-15 — 17:11, Karol Herbst wrote:> v2: make message about relaxed throttling an info > rework reporting about current clk state > > Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > drm/nouveau/include/nvkm/subdev/clk.h | 1 + > drm/nouveau/nvkm/subdev/clk/base.c | 44 +++++++++++++++++++++++++---------- > 2 files changed, 33 insertions(+), 12 deletions(-) > > diff --git a/drm/nouveau/include/nvkm/subdev/clk.h b/drm/nouveau/include/nvkm/subdev/clk.h > index f5ff1fd9..b79bf657 100644 > --- a/drm/nouveau/include/nvkm/subdev/clk.h > +++ b/drm/nouveau/include/nvkm/subdev/clk.h > @@ -106,6 +106,7 @@ struct nvkm_clk { > u8 temp; > u8 max_temp; > u8 relax_temp; > + bool throttled; > > bool allow_reclock; > #define NVKM_CLK_BOOST_NONE 0x0 > diff --git a/drm/nouveau/nvkm/subdev/clk/base.c b/drm/nouveau/nvkm/subdev/clk/base.c > index 54e14936..72ca5e0c 100644 > --- a/drm/nouveau/nvkm/subdev/clk/base.c > +++ b/drm/nouveau/nvkm/subdev/clk/base.c > @@ -279,7 +279,7 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid) > struct nvkm_fb *fb = subdev->device->fb; > struct nvkm_pci *pci = subdev->device->pci; > struct nvkm_pstate *pstate; > - int ret; > + int ret, cstate; > > if (pstateid == NVKM_CLK_PSTATE_DEFAULT) > return 0; > @@ -308,7 +308,12 @@ nvkm_pstate_prog(struct nvkm_clk *clk, int pstateid) > ram->func->tidy(ram); > } > > - return nvkm_cstate_prog(clk, pstate, clk->exp_cstateid); > + if (clk->throttled) > + cstate = list_first_entry(&pstate->list, struct nvkm_cstate, head)->id; > + else > + cstate = clk->exp_cstateid; > + > + return nvkm_cstate_prog(clk, pstate, cstate); > } > > static void > @@ -322,22 +327,20 @@ 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 C %d T %d°C\n", > - pstate, clk->pwrsrc, clk->ustate_ac, clk->ustate_dc, > - clk->astate, clk->exp_cstateid, clk->temp); > - > pstate = clk->pwrsrc ? clk->ustate_ac : clk->ustate_dc; > if (clk->state_nr && pstate != -1) { > - pstate = (pstate < 0) ? clk->astate : pstate; > + if (clk->throttled) > + pstate = list_first_entry(&clk->states, struct nvkm_pstate, head)->pstate; > + else > + pstate = (pstate < 0) ? clk->astate : pstate; > } else { > pstate = NVKM_CLK_PSTATE_DEFAULT; > } > > - nvkm_trace(subdev, "-> %d\n", pstate); > + nvkm_trace(subdev, "PWR %d U(AC) %d U(DC) %d A %d C %d T %d°C\n", > + clk->pwrsrc, clk->ustate_ac, clk->ustate_dc, > + clk->astate, clk->exp_cstateid, clk->temp); > + > if (!clk->pstate || pstate != clk->pstate->pstate) { > int ret = nvkm_pstate_prog(clk, pstate); > if (ret) { > @@ -564,9 +567,25 @@ nvkm_clk_astate(struct nvkm_clk *clk, int req, int rel, bool wait) > int > nvkm_clk_tstate(struct nvkm_clk *clk, u8 temp) > { > + struct nvkm_subdev *subdev = &clk->subdev; > if (clk->temp == temp) > return 0; > clk->temp = temp; > + if (clk->max_temp && clk->relax_temp) { > + if (!clk->throttled && temp > clk->max_temp) { > + nvkm_warn(subdev, > + "temperature (%d C) hit the 'downclock' " > + "threshold\n", > + temp); > + clk->throttled = true; > + } else if (clk->throttled && temp < clk->relax_temp) { > + nvkm_info(subdev, > + "temperature (%d C) went below the " > + "'relax' threshold\n", > + temp); > + clk->throttled = false; > + } > + } > return nvkm_clk_update(clk, false); > } > > @@ -727,6 +746,7 @@ nvkm_clk_ctor(const struct nvkm_clk_func *func, struct nvkm_device *device, > clk->ustate_dc = -1; > clk->exp_cstateid = NVKM_CLK_CSTATE_DEFAULT; > clk->temp = 90; /* reasonable default value */ > + clk->throttled = false; > > clk->allow_reclock = allow_reclock; > > -- > 2.14.1 > > _______________________________________________ > Nouveau mailing list > Nouveau at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <https://lists.freedesktop.org/archives/nouveau/attachments/20171008/61bd0660/attachment-0001.sig>
Maybe Matching Threads
- [RFC PATCH 13/13] clk: thermal throttling
- [PATCH 5/9] clk: We should pass the pstate id around not the index in the list
- [RFC PATCH 08/29] clk: We should pass the pstate id around not the index in the list
- [RFC PATCH 08/29] clk: We should pass the pstate id around not the index in the list
- [RFC PATCH 07/29] clk: Hold information about the current cstate status