[Nouveau] [PATCH 3/3] hwmon: expose power_max and power_crit

2016-11-12 Thread Karol Herbst 
Signed-off-by: Karol Herbst 
---
 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(>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(>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[] = {
_dev_attr_name.dev_attr.attr,
_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[] = {
+   _dev_attr_power1_max.dev_attr.attr,
+   _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(>rails)) {
ret = sysfs_create_group(_dev->kobj,
 _power_attrgroup);
+
if (ret)
goto error;
+
+   if (iccsense->power_w_max && iccsense->power_w_crit) {
+   ret = sysfs_create_group(_dev->kobj,
+_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->kobj, 
_fan_rpm_attrgroup);
sysfs_remove_group(>hwmon->kobj, _in0_attrgroup);
sysfs_remove_group(>hwmon->kobj, _power_attrgroup);
+   sysfs_remove_group(>hwmon->kobj, 
_power_caps_attrgroup);
 
hwmon_device_unregister(hwmon->hwmon);
}
-- 
2.10.2

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

Re: [Nouveau] [PATCH 3/3] hwmon: expose power_max and power_crit

2016-10-25 Thread Karol Herbst 


On 25 October 2016 7:33:08 a.m. GMT+02:00, Martin Peres  
wrote:
>On 25/10/16 00:11, Karol Herbst wrote:
>> Signed-off-by: Karol Herbst 
>> ---
>>  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(>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(>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[] = {
>>  _dev_attr_name.dev_attr.attr,
>>  _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[] = {
>> +_dev_attr_power1_max.dev_attr.attr,
>> +_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(>rails)) {
>>  ret = sysfs_create_group(_dev->kobj,
>>   _power_attrgroup);
>> +
>>  if (ret)
>>  goto error;
>> +
>> +if (iccsense->power_w_max && iccsense->power_w_crit) {
>> +ret = sysfs_create_group(_dev->kobj,
>> + _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->kobj, 
>> _fan_rpm_attrgroup);
>>  sysfs_remove_group(>hwmon->kobj, _in0_attrgroup);
>>  sysfs_remove_group(>hwmon->kobj, _power_attrgroup);
>> +sysfs_remove_group(>hwmon->kobj,
>_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.
>
>

no, my intention was to parse the vbios once and be done with it, otherwise we 
would spread the logic how to read the values. But maybe the entries are indeed 
always min/max/crit and there is no avg value. But the min of the cap entry 
means something like min max consumption.

Aditinally the other ebtries have really weird affects, so I would rather not 
want to parse the table every time.

There are some flags in the entries which affect what is the max allowed 
consumption and I alrwady see it will get a little complicated.

>___
>Nouveau mailing list
>Nouveau@lists.freedesktop.org

Re: [Nouveau] [PATCH 3/3] hwmon: expose power_max and power_crit

2016-10-24 Thread Martin Peres 

On 25/10/16 00:11, Karol Herbst wrote:

Signed-off-by: Karol Herbst 
---
 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(>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(>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[] = {
_dev_attr_name.dev_attr.attr,
_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[] = {
+   _dev_attr_power1_max.dev_attr.attr,
+   _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(>rails)) {
ret = sysfs_create_group(_dev->kobj,
 _power_attrgroup);
+
if (ret)
goto error;
+
+   if (iccsense->power_w_max && iccsense->power_w_crit) {
+   ret = sysfs_create_group(_dev->kobj,
+_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->kobj, 
_fan_rpm_attrgroup);
sysfs_remove_group(>hwmon->kobj, _in0_attrgroup);
sysfs_remove_group(>hwmon->kobj, _power_attrgroup);
+   sysfs_remove_group(>hwmon->kobj, 
_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.



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

[Nouveau] [PATCH 3/3] hwmon: expose power_max and power_crit

2016-10-24 Thread Karol Herbst 
Signed-off-by: Karol Herbst 
---
 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(>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(>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[] = {
_dev_attr_name.dev_attr.attr,
_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[] = {
+   _dev_attr_power1_max.dev_attr.attr,
+   _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(>rails)) {
ret = sysfs_create_group(_dev->kobj,
 _power_attrgroup);
+
if (ret)
goto error;
+
+   if (iccsense->power_w_max && iccsense->power_w_crit) {
+   ret = sysfs_create_group(_dev->kobj,
+_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->kobj, 
_fan_rpm_attrgroup);
sysfs_remove_group(>hwmon->kobj, _in0_attrgroup);
sysfs_remove_group(>hwmon->kobj, _power_attrgroup);
+   sysfs_remove_group(>hwmon->kobj, 
_power_caps_attrgroup);
 
hwmon_device_unregister(hwmon->hwmon);
}
-- 
2.10.0

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