Karol Herbst
2016-Oct-24 21:11 UTC
[Nouveau] RFC [PATCH 0/3] Expose power budget cap via hwmon
There is an optinal header field in the power budget table we can use to read out the power cap of the GPU. Sadly it is optional and if that field isn't sad, things beome complicated. Anyhow, this is good enough for most cards and we can use it later for capping the power consumption of the GPUs, but first, just export those values through hwmon. First design, will change stuff, want comments. Thanks. Karol Herbst (3): nvbios/power_budget: Add basic power budget parsing subdev/iccsense: Parse max and crit power level hwmon: expose power_max and power_crit .../include/nvkm/subdev/bios/power_budget.h | 20 ++++ drm/nouveau/include/nvkm/subdev/iccsense.h | 3 + drm/nouveau/nouveau_hwmon.c | 44 +++++++++ drm/nouveau/nvkm/subdev/bios/Kbuild | 1 + drm/nouveau/nvkm/subdev/bios/power_budget.c | 108 +++++++++++++++++++++ drm/nouveau/nvkm/subdev/iccsense/base.c | 13 ++- 6 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 drm/nouveau/include/nvkm/subdev/bios/power_budget.h create mode 100644 drm/nouveau/nvkm/subdev/bios/power_budget.c -- 2.10.0
Karol Herbst
2016-Oct-24 21:11 UTC
[Nouveau] [PATCH 1/3] nvbios/power_budget: Add basic power budget parsing
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- .../include/nvkm/subdev/bios/power_budget.h | 20 ++++ drm/nouveau/nvkm/subdev/bios/Kbuild | 1 + drm/nouveau/nvkm/subdev/bios/power_budget.c | 108 +++++++++++++++++++++ 3 files changed, 129 insertions(+) create mode 100644 drm/nouveau/include/nvkm/subdev/bios/power_budget.h create mode 100644 drm/nouveau/nvkm/subdev/bios/power_budget.c diff --git a/drm/nouveau/include/nvkm/subdev/bios/power_budget.h b/drm/nouveau/include/nvkm/subdev/bios/power_budget.h new file mode 100644 index 0000000..dd65c08 --- /dev/null +++ b/drm/nouveau/include/nvkm/subdev/bios/power_budget.h @@ -0,0 +1,20 @@ +#ifndef __NVBIOS_POWER_BUDGET_H__ +#define __NVBIOS_POWER_BUDGET_H__ + +#include <nvkm/subdev/bios.h> + +struct nvbios_power_budget_entry { + u32 min_w; + u32 avg_w; + u32 max_w; +}; + +struct nvbios_power_budget { + u8 nr_entry; + u8 cap_entry; + struct nvbios_power_budget_entry *entries; +}; + +int nvbios_power_budget_parse(struct nvkm_bios *, struct nvbios_power_budget *); + +#endif diff --git a/drm/nouveau/nvkm/subdev/bios/Kbuild b/drm/nouveau/nvkm/subdev/bios/Kbuild index be57220..6b4f1e0 100644 --- a/drm/nouveau/nvkm/subdev/bios/Kbuild +++ b/drm/nouveau/nvkm/subdev/bios/Kbuild @@ -19,6 +19,7 @@ nvkm-y += nvkm/subdev/bios/pcir.o nvkm-y += nvkm/subdev/bios/perf.o nvkm-y += nvkm/subdev/bios/pll.o nvkm-y += nvkm/subdev/bios/pmu.o +nvkm-y += nvkm/subdev/bios/power_budget.o nvkm-y += nvkm/subdev/bios/ramcfg.o nvkm-y += nvkm/subdev/bios/rammap.o nvkm-y += nvkm/subdev/bios/shadow.o diff --git a/drm/nouveau/nvkm/subdev/bios/power_budget.c b/drm/nouveau/nvkm/subdev/bios/power_budget.c new file mode 100644 index 0000000..538497b --- /dev/null +++ b/drm/nouveau/nvkm/subdev/bios/power_budget.c @@ -0,0 +1,108 @@ +/* + * Copyright 2015 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/power_budget.h> + +static u32 +nvbios_power_budget_table(struct nvkm_bios *bios, u8 *ver, u8 *hdr, u8 *cnt, + u8 *len) +{ + struct bit_entry bit_P; + u32 power_budget; + + if (bit_entry(bios, 'P', &bit_P) || bit_P.version != 2 || + bit_P.length < 0x2c) + return 0; + + power_budget = nvbios_rd32(bios, bit_P.offset + 0x2c); + if (!power_budget) + return 0; + + *ver = nvbios_rd08(bios, power_budget); + switch (*ver) { + case 0x10: + case 0x20: + case 0x30: + *hdr = nvbios_rd08(bios, power_budget + 0x1); + *len = nvbios_rd08(bios, power_budget + 0x2); + *cnt = nvbios_rd08(bios, power_budget + 0x3); + return power_budget; + default: + break; + } + + return 0; +} + +int +nvbios_power_budget_parse(struct nvkm_bios *bios, struct nvbios_power_budget *budget) +{ + struct nvkm_subdev *subdev = &bios->subdev; + u8 ver, hdr, cnt, len, i, cap_entry; + u32 header; + + header = nvbios_power_budget_table(bios, &ver, &hdr, &cnt, &len); + if (!header || !cnt) + return -ENODEV; + + budget->entries = kmalloc_array(cnt, sizeof(*budget->entries), GFP_KERNEL); + if (!budget->entries) + return -ENOMEM; + + budget->nr_entry = cnt; + switch (ver) { + case 0x20: + cap_entry = nvbios_rd08(bios, header + 0x9); + break; + default: + cap_entry = 0; + } + + if (cap_entry < cnt) + budget->cap_entry = cap_entry; + else { + if (cap_entry != 0xff) + nvkm_warn(subdev, + "invalid cap_entry in power budget table found\n"); + budget->cap_entry = 0xff; + } + + for (i = 0; i < cnt; ++i) { + u32 entry_offset = header + hdr + i * len; + struct nvbios_power_budget_entry *entry = &budget->entries[i]; + + if (len >= 0xe) { + entry->min_w = nvbios_rd32(bios, entry_offset + 0x2); + entry->avg_w = nvbios_rd32(bios, entry_offset + 0x6); + entry->max_w = nvbios_rd32(bios, entry_offset + 0xa); + } else { + entry->min_w = 0; + entry->max_w = nvbios_rd32(bios, entry_offset + 0x2); + entry->avg_w = entry->max_w; + } + } + + return 0; +} -- 2.10.0
Karol Herbst
2016-Oct-24 21:11 UTC
[Nouveau] [PATCH 2/3] subdev/iccsense: Parse max and crit power level
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/include/nvkm/subdev/iccsense.h | 3 +++ drm/nouveau/nvkm/subdev/iccsense/base.c | 13 ++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/drm/nouveau/include/nvkm/subdev/iccsense.h b/drm/nouveau/include/nvkm/subdev/iccsense.h index 3c2ddd9..b7a9b04 100644 --- a/drm/nouveau/include/nvkm/subdev/iccsense.h +++ b/drm/nouveau/include/nvkm/subdev/iccsense.h @@ -8,6 +8,9 @@ struct nvkm_iccsense { bool data_valid; struct list_head sensors; struct list_head rails; + + u32 power_w_max; + u32 power_w_crit; }; int gf100_iccsense_new(struct nvkm_device *, int index, struct nvkm_iccsense **); diff --git a/drm/nouveau/nvkm/subdev/iccsense/base.c b/drm/nouveau/nvkm/subdev/iccsense/base.c index 658355f..4e2b074 100644 --- a/drm/nouveau/nvkm/subdev/iccsense/base.c +++ b/drm/nouveau/nvkm/subdev/iccsense/base.c @@ -26,6 +26,7 @@ #include <subdev/bios.h> #include <subdev/bios/extdev.h> #include <subdev/bios/iccsense.h> +#include <subdev/bios/power_budget.h> #include <subdev/i2c.h> static bool @@ -216,10 +217,20 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev) { struct nvkm_iccsense *iccsense = nvkm_iccsense(subdev); struct nvkm_bios *bios = subdev->device->bios; + struct nvbios_power_budget budget; struct nvbios_iccsense stbl; int i; - if (!bios || nvbios_iccsense_parse(bios, &stbl) || !stbl.nr_entry) + if (!bios) + return 0; + + if (!nvbios_power_budget_parse(bios, &budget) && budget.nr_entry && + budget.cap_entry != 0xff) { + iccsense->power_w_max = budget.entries[budget.cap_entry].avg_w; + iccsense->power_w_crit = budget.entries[budget.cap_entry].max_w; + } + + if (nvbios_iccsense_parse(bios, &stbl) || !stbl.nr_entry) return 0; iccsense->data_valid = true; -- 2.10.0
Karol Herbst
2016-Oct-24 21:11 UTC
[Nouveau] [PATCH 3/3] hwmon: expose power_max and power_crit
Signed-off-by: Karol Herbst <karolherbst at gmail.com> --- drm/nouveau/nouveau_hwmon.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c index 71f764b..3d4672a 100644 --- a/drm/nouveau/nouveau_hwmon.c +++ b/drm/nouveau/nouveau_hwmon.c @@ -596,6 +596,32 @@ nouveau_hwmon_get_power1_input(struct device *d, struct device_attribute *a, static SENSOR_DEVICE_ATTR(power1_input, S_IRUGO, nouveau_hwmon_get_power1_input, NULL, 0); +static ssize_t +nouveau_hwmon_get_power1_max(struct device *d, struct device_attribute *a, + char *buf) +{ + struct drm_device *dev = dev_get_drvdata(d); + struct nouveau_drm *drm = nouveau_drm(dev); + struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->device); + return sprintf(buf, "%i\n", iccsense->power_w_max); +} + +static SENSOR_DEVICE_ATTR(power1_max, S_IRUGO, + nouveau_hwmon_get_power1_max, NULL, 0); + +static ssize_t +nouveau_hwmon_get_power1_crit(struct device *d, struct device_attribute *a, + char *buf) +{ + struct drm_device *dev = dev_get_drvdata(d); + struct nouveau_drm *drm = nouveau_drm(dev); + struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->device); + return sprintf(buf, "%i\n", iccsense->power_w_crit); +} + +static SENSOR_DEVICE_ATTR(power1_crit, S_IRUGO, + nouveau_hwmon_get_power1_crit, NULL, 0); + static struct attribute *hwmon_default_attributes[] = { &sensor_dev_attr_name.dev_attr.attr, &sensor_dev_attr_update_rate.dev_attr.attr, @@ -639,6 +665,12 @@ static struct attribute *hwmon_power_attributes[] = { NULL }; +static struct attribute *hwmon_power_caps_attributes[] = { + &sensor_dev_attr_power1_max.dev_attr.attr, + &sensor_dev_attr_power1_crit.dev_attr.attr, + NULL +}; + static const struct attribute_group hwmon_default_attrgroup = { .attrs = hwmon_default_attributes, }; @@ -657,6 +689,9 @@ static const struct attribute_group hwmon_in0_attrgroup = { static const struct attribute_group hwmon_power_attrgroup = { .attrs = hwmon_power_attributes, }; +static const struct attribute_group hwmon_power_caps_attrgroup = { + .attrs = hwmon_power_caps_attributes, +}; #endif int @@ -728,8 +763,16 @@ nouveau_hwmon_init(struct drm_device *dev) if (iccsense && iccsense->data_valid && !list_empty(&iccsense->rails)) { ret = sysfs_create_group(&hwmon_dev->kobj, &hwmon_power_attrgroup); + if (ret) goto error; + + if (iccsense->power_w_max && iccsense->power_w_crit) { + ret = sysfs_create_group(&hwmon_dev->kobj, + &hwmon_power_caps_attrgroup); + if (ret) + goto error; + } } hwmon->hwmon = hwmon_dev; @@ -759,6 +802,7 @@ nouveau_hwmon_fini(struct drm_device *dev) sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_fan_rpm_attrgroup); sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_in0_attrgroup); sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_power_attrgroup); + sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_power_caps_attrgroup); hwmon_device_unregister(hwmon->hwmon); } -- 2.10.0
Martin Peres
2016-Oct-25 05:23 UTC
[Nouveau] RFC [PATCH 0/3] Expose power budget cap via hwmon
On 25/10/16 00:11, Karol Herbst wrote:> There is an optinal header field in the power budget table we can use to > read out the power cap of the GPU. > > Sadly it is optional and if that field isn't sad, things beomeoh.... why would it be sad? Poor little one :p> complicated. > > Anyhow, this is good enough for most cards and we can use it later for > capping the power consumption of the GPUs, but first, just export those > values through hwmon. > > First design, will change stuff, want comments. Thanks.Thanks for doing this!> > Karol Herbst (3): > nvbios/power_budget: Add basic power budget parsing > subdev/iccsense: Parse max and crit power level > hwmon: expose power_max and power_crit > > .../include/nvkm/subdev/bios/power_budget.h | 20 ++++ > drm/nouveau/include/nvkm/subdev/iccsense.h | 3 + > drm/nouveau/nouveau_hwmon.c | 44 +++++++++ > drm/nouveau/nvkm/subdev/bios/Kbuild | 1 + > drm/nouveau/nvkm/subdev/bios/power_budget.c | 108 +++++++++++++++++++++ > drm/nouveau/nvkm/subdev/iccsense/base.c | 13 ++- > 6 files changed, 188 insertions(+), 1 deletion(-) > create mode 100644 drm/nouveau/include/nvkm/subdev/bios/power_budget.h > create mode 100644 drm/nouveau/nvkm/subdev/bios/power_budget.c >
Martin Peres
2016-Oct-25 05:25 UTC
[Nouveau] [PATCH 1/3] nvbios/power_budget: Add basic power budget parsing
On 25/10/16 00:11, Karol Herbst wrote:> Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > .../include/nvkm/subdev/bios/power_budget.h | 20 ++++ > drm/nouveau/nvkm/subdev/bios/Kbuild | 1 + > drm/nouveau/nvkm/subdev/bios/power_budget.c | 108 +++++++++++++++++++++ > 3 files changed, 129 insertions(+) > create mode 100644 drm/nouveau/include/nvkm/subdev/bios/power_budget.h > create mode 100644 drm/nouveau/nvkm/subdev/bios/power_budget.c > > diff --git a/drm/nouveau/include/nvkm/subdev/bios/power_budget.h b/drm/nouveau/include/nvkm/subdev/bios/power_budget.h > new file mode 100644 > index 0000000..dd65c08 > --- /dev/null > +++ b/drm/nouveau/include/nvkm/subdev/bios/power_budget.h > @@ -0,0 +1,20 @@ > +#ifndef __NVBIOS_POWER_BUDGET_H__ > +#define __NVBIOS_POWER_BUDGET_H__ > + > +#include <nvkm/subdev/bios.h> > + > +struct nvbios_power_budget_entry { > + u32 min_w; > + u32 avg_w; > + u32 max_w; > +}; > + > +struct nvbios_power_budget { > + u8 nr_entry; > + u8 cap_entry; > + struct nvbios_power_budget_entry *entries; > +}; > + > +int nvbios_power_budget_parse(struct nvkm_bios *, struct nvbios_power_budget *); > + > +#endif > diff --git a/drm/nouveau/nvkm/subdev/bios/Kbuild b/drm/nouveau/nvkm/subdev/bios/Kbuild > index be57220..6b4f1e0 100644 > --- a/drm/nouveau/nvkm/subdev/bios/Kbuild > +++ b/drm/nouveau/nvkm/subdev/bios/Kbuild > @@ -19,6 +19,7 @@ nvkm-y += nvkm/subdev/bios/pcir.o > nvkm-y += nvkm/subdev/bios/perf.o > nvkm-y += nvkm/subdev/bios/pll.o > nvkm-y += nvkm/subdev/bios/pmu.o > +nvkm-y += nvkm/subdev/bios/power_budget.o > nvkm-y += nvkm/subdev/bios/ramcfg.o > nvkm-y += nvkm/subdev/bios/rammap.o > nvkm-y += nvkm/subdev/bios/shadow.o > diff --git a/drm/nouveau/nvkm/subdev/bios/power_budget.c b/drm/nouveau/nvkm/subdev/bios/power_budget.c > new file mode 100644 > index 0000000..538497b > --- /dev/null > +++ b/drm/nouveau/nvkm/subdev/bios/power_budget.c > @@ -0,0 +1,108 @@ > +/* > + * Copyright 2015 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/power_budget.h> > + > +static u32 > +nvbios_power_budget_table(struct nvkm_bios *bios, u8 *ver, u8 *hdr, u8 *cnt, > + u8 *len) > +{ > + struct bit_entry bit_P; > + u32 power_budget; > + > + if (bit_entry(bios, 'P', &bit_P) || bit_P.version != 2 || > + bit_P.length < 0x2c) > + return 0; > + > + power_budget = nvbios_rd32(bios, bit_P.offset + 0x2c); > + if (!power_budget) > + return 0; > + > + *ver = nvbios_rd08(bios, power_budget); > + switch (*ver) { > + case 0x10: > + case 0x20: > + case 0x30: > + *hdr = nvbios_rd08(bios, power_budget + 0x1); > + *len = nvbios_rd08(bios, power_budget + 0x2); > + *cnt = nvbios_rd08(bios, power_budget + 0x3); > + return power_budget; > + default: > + break; > + } > + > + return 0; > +} > + > +int > +nvbios_power_budget_parse(struct nvkm_bios *bios, struct nvbios_power_budget *budget) > +{ > + struct nvkm_subdev *subdev = &bios->subdev; > + u8 ver, hdr, cnt, len, i, cap_entry; > + u32 header; > + > + header = nvbios_power_budget_table(bios, &ver, &hdr, &cnt, &len); > + if (!header || !cnt) > + return -ENODEV; > + > + budget->entries = kmalloc_array(cnt, sizeof(*budget->entries), GFP_KERNEL); > + if (!budget->entries) > + return -ENOMEM; > + > + budget->nr_entry = cnt; > + switch (ver) { > + case 0x20: > + cap_entry = nvbios_rd08(bios, header + 0x9); > + break; > + default: > + cap_entry = 0;Are you sure about this? How about setting it to 0xff instead?> + } > + > + if (cap_entry < cnt) > + budget->cap_entry = cap_entry; > + else { > + if (cap_entry != 0xff) > + nvkm_warn(subdev, > + "invalid cap_entry in power budget table found\n"); > + budget->cap_entry = 0xff; > + } > + > + for (i = 0; i < cnt; ++i) { > + u32 entry_offset = header + hdr + i * len; > + struct nvbios_power_budget_entry *entry = &budget->entries[i]; > + > + if (len >= 0xe) { > + entry->min_w = nvbios_rd32(bios, entry_offset + 0x2); > + entry->avg_w = nvbios_rd32(bios, entry_offset + 0x6); > + entry->max_w = nvbios_rd32(bios, entry_offset + 0xa); > + } else { > + entry->min_w = 0; > + entry->max_w = nvbios_rd32(bios, entry_offset + 0x2); > + entry->avg_w = entry->max_w; > + } > + } > + > + return 0; > +} >
Martin Peres
2016-Oct-25 05:28 UTC
[Nouveau] [PATCH 2/3] subdev/iccsense: Parse max and crit power level
On 25/10/16 00:11, Karol Herbst wrote:> Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > drm/nouveau/include/nvkm/subdev/iccsense.h | 3 +++ > drm/nouveau/nvkm/subdev/iccsense/base.c | 13 ++++++++++++- > 2 files changed, 15 insertions(+), 1 deletion(-)What is the point of duplicating values here? Just store the parsed bios table, like we do for fan management.
Martin Peres
2016-Oct-25 05:33 UTC
[Nouveau] [PATCH 3/3] hwmon: expose power_max and power_crit
On 25/10/16 00:11, Karol Herbst wrote:> Signed-off-by: Karol Herbst <karolherbst at gmail.com> > --- > drm/nouveau/nouveau_hwmon.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/drm/nouveau/nouveau_hwmon.c b/drm/nouveau/nouveau_hwmon.c > index 71f764b..3d4672a 100644 > --- a/drm/nouveau/nouveau_hwmon.c > +++ b/drm/nouveau/nouveau_hwmon.c > @@ -596,6 +596,32 @@ nouveau_hwmon_get_power1_input(struct device *d, struct device_attribute *a, > static SENSOR_DEVICE_ATTR(power1_input, S_IRUGO, > nouveau_hwmon_get_power1_input, NULL, 0); > > +static ssize_t > +nouveau_hwmon_get_power1_max(struct device *d, struct device_attribute *a, > + char *buf) > +{ > + struct drm_device *dev = dev_get_drvdata(d); > + struct nouveau_drm *drm = nouveau_drm(dev); > + struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->device); > + return sprintf(buf, "%i\n", iccsense->power_w_max); > +} > + > +static SENSOR_DEVICE_ATTR(power1_max, S_IRUGO, > + nouveau_hwmon_get_power1_max, NULL, 0); > + > +static ssize_t > +nouveau_hwmon_get_power1_crit(struct device *d, struct device_attribute *a, > + char *buf) > +{ > + struct drm_device *dev = dev_get_drvdata(d); > + struct nouveau_drm *drm = nouveau_drm(dev); > + struct nvkm_iccsense *iccsense = nvxx_iccsense(&drm->device); > + return sprintf(buf, "%i\n", iccsense->power_w_crit); > +} > + > +static SENSOR_DEVICE_ATTR(power1_crit, S_IRUGO, > + nouveau_hwmon_get_power1_crit, NULL, 0); > + > static struct attribute *hwmon_default_attributes[] = { > &sensor_dev_attr_name.dev_attr.attr, > &sensor_dev_attr_update_rate.dev_attr.attr, > @@ -639,6 +665,12 @@ static struct attribute *hwmon_power_attributes[] = { > NULL > }; > > +static struct attribute *hwmon_power_caps_attributes[] = { > + &sensor_dev_attr_power1_max.dev_attr.attr, > + &sensor_dev_attr_power1_crit.dev_attr.attr, > + NULL > +}; > + > static const struct attribute_group hwmon_default_attrgroup = { > .attrs = hwmon_default_attributes, > }; > @@ -657,6 +689,9 @@ static const struct attribute_group hwmon_in0_attrgroup = { > static const struct attribute_group hwmon_power_attrgroup = { > .attrs = hwmon_power_attributes, > }; > +static const struct attribute_group hwmon_power_caps_attrgroup = { > + .attrs = hwmon_power_caps_attributes, > +}; > #endif > > int > @@ -728,8 +763,16 @@ nouveau_hwmon_init(struct drm_device *dev) > if (iccsense && iccsense->data_valid && !list_empty(&iccsense->rails)) { > ret = sysfs_create_group(&hwmon_dev->kobj, > &hwmon_power_attrgroup); > + > if (ret) > goto error; > + > + if (iccsense->power_w_max && iccsense->power_w_crit) { > + ret = sysfs_create_group(&hwmon_dev->kobj, > + &hwmon_power_caps_attrgroup); > + if (ret) > + goto error; > + } > } > > hwmon->hwmon = hwmon_dev; > @@ -759,6 +802,7 @@ nouveau_hwmon_fini(struct drm_device *dev) > sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_fan_rpm_attrgroup); > sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_in0_attrgroup); > sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_power_attrgroup); > + sysfs_remove_group(&hwmon->hwmon->kobj, &hwmon_power_caps_attrgroup); > > hwmon_device_unregister(hwmon->hwmon); > } >So, you wanted to make a cache because you felt you may want to let the user edit the values? Not sure we should ever allow this :s At least, a RO file is a very good idea to help debugging and just letting the user know about such limitations. So, in my opinion, I think we should just kill patch 2 and use directly the output of the bios table here. Maybe you can write a helper function in patch 1 to get the min, avg and max values and return -1 if it is not valid? This way, you can access this info very easily in multiple places without duplicating this 0xff check.
Reasonably Related Threads
- [PATCH 3/3] hwmon: expose power_max and power_crit
- [PATCH v5 3/5] nouveau_hwmon: Remove old code, add .write/.read operations
- [PATCH 2/4] nouveau_hwmon: migrate to hwmon_device_register_with_info
- [PATCH 1/1] nouveau_hwmon: migrate to hwmon_device_register_with_info
- [PATCH v4 4/6] hwmon: add power consumption