[PATCH 1/2] drm/amd/powerplay: export a function to read fan rpm

[PATCH 1/2] drm/amd/powerplay: export a function to read fan rpm

[Grazvydas Ignotas]

Powerplay hwmgr already has an implementation, all we need to do is to call it.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c     | 18 ++++++++++++++++++
 drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
index 0b1f220..1f49764 100644
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
@@ -582,6 +582,23 @@ static int pp_dpm_get_fan_speed_percent(void *handle, uint32_t *speed)
 	return hwmgr->hwmgr_func->get_fan_speed_percent(hwmgr, speed);
 }
 
+static int pp_dpm_get_fan_speed_rpm(void *handle, uint32_t *rpm)
+{
+	struct pp_hwmgr *hwmgr;
+
+	if (handle == NULL)
+		return -EINVAL;
+
+	hwmgr = ((struct pp_instance *)handle)->hwmgr;
+
+	PP_CHECK_HW(hwmgr);
+
+	if (hwmgr->hwmgr_func->get_fan_speed_rpm == NULL)
+		return -EINVAL;
+
+	return hwmgr->hwmgr_func->get_fan_speed_rpm(hwmgr, rpm);
+}
+
 static int pp_dpm_get_temperature(void *handle)
 {
 	struct pp_hwmgr  *hwmgr;
@@ -852,6 +869,7 @@ const struct amd_powerplay_funcs pp_dpm_funcs = {
 	.get_fan_control_mode = pp_dpm_get_fan_control_mode,
 	.set_fan_speed_percent = pp_dpm_set_fan_speed_percent,
 	.get_fan_speed_percent = pp_dpm_get_fan_speed_percent,
+	.get_fan_speed_rpm = pp_dpm_get_fan_speed_rpm,
 	.get_pp_num_states = pp_dpm_get_pp_num_states,
 	.get_pp_table = pp_dpm_get_pp_table,
 	.set_pp_table = pp_dpm_set_pp_table,
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
index eb3e83d..2892b4e 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
@@ -349,6 +349,7 @@ struct amd_powerplay_funcs {
 	int (*get_fan_control_mode)(void *handle);
 	int (*set_fan_speed_percent)(void *handle, uint32_t percent);
 	int (*get_fan_speed_percent)(void *handle, uint32_t *speed);
+	int (*get_fan_speed_rpm)(void *handle, uint32_t *rpm);
 	int (*get_pp_num_states)(void *handle, struct pp_states_info *data);
 	int (*get_pp_table)(void *handle, char **table);
 	int (*set_pp_table)(void *handle, const char *buf, size_t size);
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amd/amdgpu: expose fan rpm though hwmon

[Grazvydas Ignotas]

Only for cards that are supported by powerplay.

Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h |  5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c  | 21 +++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
index bd85e35..e45bd05 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
@@ -317,6 +317,11 @@ struct amdgpu_dpm_funcs {
 	      (adev)->powerplay.pp_funcs->get_fan_speed_percent((adev)->powerplay.pp_handle, (s)) : \
 	      (adev)->pm.funcs->get_fan_speed_percent((adev), (s)))
 
+#define amdgpu_dpm_get_fan_speed_rpm(adev, s) \
+	((adev)->pp_enabled ?						\
+	      (adev)->powerplay.pp_funcs->get_fan_speed_rpm((adev)->powerplay.pp_handle, (s)) : \
+	      -EINVAL)
+
 #define amdgpu_dpm_get_sclk(adev, l) \
 	((adev)->pp_enabled ?						\
 	      (adev)->powerplay.pp_funcs->get_sclk((adev)->powerplay.pp_handle, (l)) : \
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 274f330..723ae68 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -737,6 +737,21 @@ static ssize_t amdgpu_hwmon_get_pwm1(struct device *dev,
 	return sprintf(buf, "%i\n", speed);
 }
 
+static ssize_t amdgpu_hwmon_get_fan1_input(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct amdgpu_device *adev = dev_get_drvdata(dev);
+	int err;
+	u32 speed;
+
+	err = amdgpu_dpm_get_fan_speed_rpm(adev, &speed);
+	if (err)
+		return err;
+
+	return sprintf(buf, "%i\n", speed);
+}
+
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, amdgpu_hwmon_show_temp, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, amdgpu_hwmon_show_temp_thresh, NULL, 0);
 static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IRUGO, amdgpu_hwmon_show_temp_thresh, NULL, 1);
@@ -744,6 +759,7 @@ static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, amdgpu_hwmon_get_pwm1, amdgpu
 static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO | S_IWUSR, amdgpu_hwmon_get_pwm1_enable, amdgpu_hwmon_set_pwm1_enable, 0);
 static SENSOR_DEVICE_ATTR(pwm1_min, S_IRUGO, amdgpu_hwmon_get_pwm1_min, NULL, 0);
 static SENSOR_DEVICE_ATTR(pwm1_max, S_IRUGO, amdgpu_hwmon_get_pwm1_max, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, amdgpu_hwmon_get_fan1_input, NULL, 0);
 
 static struct attribute *hwmon_attributes[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
@@ -753,6 +769,7 @@ static struct attribute *hwmon_attributes[] = {
 	&sensor_dev_attr_pwm1_enable.dev_attr.attr,
 	&sensor_dev_attr_pwm1_min.dev_attr.attr,
 	&sensor_dev_attr_pwm1_max.dev_attr.attr,
+	&sensor_dev_attr_fan1_input.dev_attr.attr,
 	NULL
 };
 
@@ -804,6 +821,10 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
 	     attr == &sensor_dev_attr_pwm1_min.dev_attr.attr))
 		return 0;
 
+	/* requires powerplay */
+	if (attr == &sensor_dev_attr_fan1_input.dev_attr.attr)
+		return 0;
+
 	return effective_mode;
 }
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/amdgpu: expose fan rpm though hwmon

[Alex Deucher]

On Sat, Oct 29, 2016 at 4:28 PM, Grazvydas Ignotas <notasas@gmail.com> wrote:
> Only for cards that are supported by powerplay.
>
> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>

Applied the series.  Thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h |  5 +++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c  | 21 +++++++++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> index bd85e35..e45bd05 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> @@ -317,6 +317,11 @@ struct amdgpu_dpm_funcs {
>               (adev)->powerplay.pp_funcs->get_fan_speed_percent((adev)->powerplay.pp_handle, (s)) : \
>               (adev)->pm.funcs->get_fan_speed_percent((adev), (s)))
>
> +#define amdgpu_dpm_get_fan_speed_rpm(adev, s) \
> +       ((adev)->pp_enabled ?                                           \
> +             (adev)->powerplay.pp_funcs->get_fan_speed_rpm((adev)->powerplay.pp_handle, (s)) : \
> +             -EINVAL)
> +
>  #define amdgpu_dpm_get_sclk(adev, l) \
>         ((adev)->pp_enabled ?                                           \
>               (adev)->powerplay.pp_funcs->get_sclk((adev)->powerplay.pp_handle, (l)) : \
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 274f330..723ae68 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -737,6 +737,21 @@ static ssize_t amdgpu_hwmon_get_pwm1(struct device *dev,
>         return sprintf(buf, "%i\n", speed);
>  }
>
> +static ssize_t amdgpu_hwmon_get_fan1_input(struct device *dev,
> +                                          struct device_attribute *attr,
> +                                          char *buf)
> +{
> +       struct amdgpu_device *adev = dev_get_drvdata(dev);
> +       int err;
> +       u32 speed;
> +
> +       err = amdgpu_dpm_get_fan_speed_rpm(adev, &speed);
> +       if (err)
> +               return err;
> +
> +       return sprintf(buf, "%i\n", speed);
> +}
> +
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, amdgpu_hwmon_show_temp, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, amdgpu_hwmon_show_temp_thresh, NULL, 0);
>  static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IRUGO, amdgpu_hwmon_show_temp_thresh, NULL, 1);
> @@ -744,6 +759,7 @@ static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, amdgpu_hwmon_get_pwm1, amdgpu
>  static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO | S_IWUSR, amdgpu_hwmon_get_pwm1_enable, amdgpu_hwmon_set_pwm1_enable, 0);
>  static SENSOR_DEVICE_ATTR(pwm1_min, S_IRUGO, amdgpu_hwmon_get_pwm1_min, NULL, 0);
>  static SENSOR_DEVICE_ATTR(pwm1_max, S_IRUGO, amdgpu_hwmon_get_pwm1_max, NULL, 0);
> +static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, amdgpu_hwmon_get_fan1_input, NULL, 0);
>
>  static struct attribute *hwmon_attributes[] = {
>         &sensor_dev_attr_temp1_input.dev_attr.attr,
> @@ -753,6 +769,7 @@ static struct attribute *hwmon_attributes[] = {
>         &sensor_dev_attr_pwm1_enable.dev_attr.attr,
>         &sensor_dev_attr_pwm1_min.dev_attr.attr,
>         &sensor_dev_attr_pwm1_max.dev_attr.attr,
> +       &sensor_dev_attr_fan1_input.dev_attr.attr,
>         NULL
>  };
>
> @@ -804,6 +821,10 @@ static umode_t hwmon_attributes_visible(struct kobject *kobj,
>              attr == &sensor_dev_attr_pwm1_min.dev_attr.attr))
>                 return 0;
>
> +       /* requires powerplay */
> +       if (attr == &sensor_dev_attr_fan1_input.dev_attr.attr)
> +               return 0;
> +
>         return effective_mode;
>  }
>
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amd/powerplay: export a function to read fan rpm

[Edward O'Callaghan]

[-- Attachment #1.1.1: Type: text/plain, Size: 2718 bytes --]

Howdy,

On 10/30/2016 07:28 AM, Grazvydas Ignotas wrote:
> Powerplay hwmgr already has an implementation, all we need to do is to call it.
> 
> Signed-off-by: Grazvydas Ignotas <notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c     | 18 ++++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h |  1 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index 0b1f220..1f49764 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -582,6 +582,23 @@ static int pp_dpm_get_fan_speed_percent(void *handle, uint32_t *speed)
>  	return hwmgr->hwmgr_func->get_fan_speed_percent(hwmgr, speed);
>  }
>  
> +static int pp_dpm_get_fan_speed_rpm(void *handle, uint32_t *rpm)

why not type the handle rather than have 'void *' and a coercion (I
didn't check the call site yet..)

> +{
> +	struct pp_hwmgr *hwmgr;
> +
> +	if (handle == NULL)

if (!handle)

> +		return -EINVAL;
> +
> +	hwmgr = ((struct pp_instance *)handle)->hwmgr;
> +
> +	PP_CHECK_HW(hwmgr);
> +
> +	if (hwmgr->hwmgr_func->get_fan_speed_rpm == NULL)

if (!hwmgr->hwmgr_func->get_fan_speed_rpm)

> +		return -EINVAL;
> +
> +	return hwmgr->hwmgr_func->get_fan_speed_rpm(hwmgr, rpm);
> +}
> +
>  static int pp_dpm_get_temperature(void *handle)
>  {
>  	struct pp_hwmgr  *hwmgr;
> @@ -852,6 +869,7 @@ const struct amd_powerplay_funcs pp_dpm_funcs = {
>  	.get_fan_control_mode = pp_dpm_get_fan_control_mode,
>  	.set_fan_speed_percent = pp_dpm_set_fan_speed_percent,
>  	.get_fan_speed_percent = pp_dpm_get_fan_speed_percent,
> +	.get_fan_speed_rpm = pp_dpm_get_fan_speed_rpm,
>  	.get_pp_num_states = pp_dpm_get_pp_num_states,
>  	.get_pp_table = pp_dpm_get_pp_table,
>  	.set_pp_table = pp_dpm_set_pp_table,
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> index eb3e83d..2892b4e 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> @@ -349,6 +349,7 @@ struct amd_powerplay_funcs {
>  	int (*get_fan_control_mode)(void *handle);
>  	int (*set_fan_speed_percent)(void *handle, uint32_t percent);
>  	int (*get_fan_speed_percent)(void *handle, uint32_t *speed);
> +	int (*get_fan_speed_rpm)(void *handle, uint32_t *rpm);
>  	int (*get_pp_num_states)(void *handle, struct pp_states_info *data);
>  	int (*get_pp_table)(void *handle, char **table);
>  	int (*set_pp_table)(void *handle, const char *buf, size_t size);
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amd/powerplay: export a function to read fan rpm

[Grazvydas Ignotas]

Hi,

On Sun, Oct 30, 2016 at 7:10 AM, Edward O'Callaghan
<funfunctor@folklore1984.net> wrote:
> Howdy,
>
> On 10/30/2016 07:28 AM, Grazvydas Ignotas wrote:
>> Powerplay hwmgr already has an implementation, all we need to do is to call it.
>>
>> Signed-off-by: Grazvydas Ignotas <notasas@gmail.com>
>> ---
>>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c     | 18 ++++++++++++++++++
>>  drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h |  1 +
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> index 0b1f220..1f49764 100644
>> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> @@ -582,6 +582,23 @@ static int pp_dpm_get_fan_speed_percent(void *handle, uint32_t *speed)
>>       return hwmgr->hwmgr_func->get_fan_speed_percent(hwmgr, speed);
>>  }
>>
>> +static int pp_dpm_get_fan_speed_rpm(void *handle, uint32_t *rpm)
>
> why not type the handle rather than have 'void *' and a coercion (I
> didn't check the call site yet..)

Well the rest of this file is written in such a style, as well as all
the other function pointers having void * handles in the powerplay
table, so it would be an odd one standing out from the others. Same
thing for the NULL checks.

>
>> +{
>> +     struct pp_hwmgr *hwmgr;
>> +
>> +     if (handle == NULL)
>
> if (!handle)
>
>> +             return -EINVAL;
>> +
>> +     hwmgr = ((struct pp_instance *)handle)->hwmgr;
>> +
>> +     PP_CHECK_HW(hwmgr);
>> +
>> +     if (hwmgr->hwmgr_func->get_fan_speed_rpm == NULL)
>
> if (!hwmgr->hwmgr_func->get_fan_speed_rpm)
>
>> +             return -EINVAL;
>> +
>> +     return hwmgr->hwmgr_func->get_fan_speed_rpm(hwmgr, rpm);
>> +}
>> +
>>  static int pp_dpm_get_temperature(void *handle)
>>  {
>>       struct pp_hwmgr  *hwmgr;
>> @@ -852,6 +869,7 @@ const struct amd_powerplay_funcs pp_dpm_funcs = {
>>       .get_fan_control_mode = pp_dpm_get_fan_control_mode,
>>       .set_fan_speed_percent = pp_dpm_set_fan_speed_percent,
>>       .get_fan_speed_percent = pp_dpm_get_fan_speed_percent,
>> +     .get_fan_speed_rpm = pp_dpm_get_fan_speed_rpm,
>>       .get_pp_num_states = pp_dpm_get_pp_num_states,
>>       .get_pp_table = pp_dpm_get_pp_table,
>>       .set_pp_table = pp_dpm_set_pp_table,
>> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> index eb3e83d..2892b4e 100644
>> --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> @@ -349,6 +349,7 @@ struct amd_powerplay_funcs {
>>       int (*get_fan_control_mode)(void *handle);
>>       int (*set_fan_speed_percent)(void *handle, uint32_t percent);
>>       int (*get_fan_speed_percent)(void *handle, uint32_t *speed);
>> +     int (*get_fan_speed_rpm)(void *handle, uint32_t *rpm);
>>       int (*get_pp_num_states)(void *handle, struct pp_states_info *data);
>>       int (*get_pp_table)(void *handle, char **table);
>>       int (*set_pp_table)(void *handle, const char *buf, size_t size);
>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx