All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/radeon: use 0-255 rather than 0-100 for pwm fan range
@ 2015-02-04 22:27 Alex Deucher
  2015-02-05  9:49 ` Christian König
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2015-02-04 22:27 UTC (permalink / raw)
  To: dri-devel; +Cc: Alex Deucher

0-255 seems to be the preferred range for the pwm interface.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_pm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
index 91e1bd2..9f758d3 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -585,7 +585,7 @@ static ssize_t radeon_hwmon_set_pwm1_enable(struct device *dev,
 	if (err)
 		return err;
 
-	switch(value) {
+	switch (value) {
 	case 1: /* manual, percent-based */
 		rdev->asic->dpm.fan_ctrl_set_mode(rdev, FDO_PWM_MODE_STATIC);
 		break;
@@ -608,7 +608,7 @@ static ssize_t radeon_hwmon_get_pwm1_max(struct device *dev,
 					 struct device_attribute *attr,
 					 char *buf)
 {
-	return sprintf(buf, "%i\n", 100); /* pwm uses percent-based fan-control */
+	return sprintf(buf, "%i\n", 255);
 }
 
 static ssize_t radeon_hwmon_set_pwm1(struct device *dev,
@@ -623,6 +623,8 @@ static ssize_t radeon_hwmon_set_pwm1(struct device *dev,
 	if (err)
 		return err;
 
+	value = (value * 100) / 255;
+
 	err = rdev->asic->dpm.set_fan_speed_percent(rdev, value);
 	if (err)
 		return err;
@@ -642,6 +644,8 @@ static ssize_t radeon_hwmon_get_pwm1(struct device *dev,
 	if (err)
 		return err;
 
+	speed = (speed * 255) / 100;
+
 	return sprintf(buf, "%i\n", speed);
 }
 
-- 
1.8.3.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/radeon: use 0-255 rather than 0-100 for pwm fan range
  2015-02-04 22:27 [PATCH] drm/radeon: use 0-255 rather than 0-100 for pwm fan range Alex Deucher
@ 2015-02-05  9:49 ` Christian König
  2015-04-11  9:52   ` Martin Peres
  0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2015-02-05  9:49 UTC (permalink / raw)
  To: Alex Deucher, dri-devel; +Cc: Alex Deucher

Am 04.02.2015 um 23:27 schrieb Alex Deucher:
> 0-255 seems to be the preferred range for the pwm interface.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Yeah, using 100 on a 8bit pwm timer sounds rather obviously wrong.

Patch is Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/radeon/radeon_pm.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
> index 91e1bd2..9f758d3 100644
> --- a/drivers/gpu/drm/radeon/radeon_pm.c
> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
> @@ -585,7 +585,7 @@ static ssize_t radeon_hwmon_set_pwm1_enable(struct device *dev,
>   	if (err)
>   		return err;
>   
> -	switch(value) {
> +	switch (value) {
>   	case 1: /* manual, percent-based */
>   		rdev->asic->dpm.fan_ctrl_set_mode(rdev, FDO_PWM_MODE_STATIC);
>   		break;
> @@ -608,7 +608,7 @@ static ssize_t radeon_hwmon_get_pwm1_max(struct device *dev,
>   					 struct device_attribute *attr,
>   					 char *buf)
>   {
> -	return sprintf(buf, "%i\n", 100); /* pwm uses percent-based fan-control */
> +	return sprintf(buf, "%i\n", 255);
>   }
>   
>   static ssize_t radeon_hwmon_set_pwm1(struct device *dev,
> @@ -623,6 +623,8 @@ static ssize_t radeon_hwmon_set_pwm1(struct device *dev,
>   	if (err)
>   		return err;
>   
> +	value = (value * 100) / 255;
> +
>   	err = rdev->asic->dpm.set_fan_speed_percent(rdev, value);
>   	if (err)
>   		return err;
> @@ -642,6 +644,8 @@ static ssize_t radeon_hwmon_get_pwm1(struct device *dev,
>   	if (err)
>   		return err;
>   
> +	speed = (speed * 255) / 100;
> +
>   	return sprintf(buf, "%i\n", speed);
>   }
>   

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/radeon: use 0-255 rather than 0-100 for pwm fan range
  2015-02-05  9:49 ` Christian König
@ 2015-04-11  9:52   ` Martin Peres
  2015-04-12 23:42     ` Alex Deucher
  0 siblings, 1 reply; 5+ messages in thread
From: Martin Peres @ 2015-04-11  9:52 UTC (permalink / raw)
  To: dri-devel

On 05/02/2015 11:49, Christian König wrote:
> Am 04.02.2015 um 23:27 schrieb Alex Deucher:
>> 0-255 seems to be the preferred range for the pwm interface.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Yeah, using 100 on a 8bit pwm timer sounds rather obviously wrong.
>
> Patch is Reviewed-by: Christian König <christian.koenig@amd.com>
>
>> ---
>>    drivers/gpu/drm/radeon/radeon_pm.c | 8 ++++++--
>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
>> index 91e1bd2..9f758d3 100644
>> --- a/drivers/gpu/drm/radeon/radeon_pm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
>> @@ -585,7 +585,7 @@ static ssize_t radeon_hwmon_set_pwm1_enable(struct device *dev,
>>    	if (err)
>>    		return err;
>>    
>> -	switch(value) {
>> +	switch (value) {
>>    	case 1: /* manual, percent-based */
>>    		rdev->asic->dpm.fan_ctrl_set_mode(rdev, FDO_PWM_MODE_STATIC);
>>    		break;
>> @@ -608,7 +608,7 @@ static ssize_t radeon_hwmon_get_pwm1_max(struct device *dev,
>>    					 struct device_attribute *attr,
>>    					 char *buf)
>>    {
>> -	return sprintf(buf, "%i\n", 100); /* pwm uses percent-based fan-control */
>> +	return sprintf(buf, "%i\n", 255);
>>    }

This is not a standard name as it is not documented in 
https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface

Apparently, the pwm value should always have been exposed as 0-255 and I 
screwed it up in nouveau by having it be 0-100...

Maybe we should ask pwm*_max to be defined so as new applications could 
do the right thing while not having nouveau change its ABI. I guess it 
is ok to change it for radeon as there are so few users currently but 
the interface has been in nouveau for multiple years already!

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/radeon: use 0-255 rather than 0-100 for pwm fan range
  2015-04-11  9:52   ` Martin Peres
@ 2015-04-12 23:42     ` Alex Deucher
  2015-04-14 21:19       ` Martin Peres
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2015-04-12 23:42 UTC (permalink / raw)
  To: Martin Peres; +Cc: Maling list - DRI developers

On Sat, Apr 11, 2015 at 5:52 AM, Martin Peres <martin.peres@free.fr> wrote:
> On 05/02/2015 11:49, Christian König wrote:
>>
>> Am 04.02.2015 um 23:27 schrieb Alex Deucher:
>>>
>>> 0-255 seems to be the preferred range for the pwm interface.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> Yeah, using 100 on a 8bit pwm timer sounds rather obviously wrong.
>>
>> Patch is Reviewed-by: Christian König <christian.koenig@amd.com>
>>
>>> ---
>>>    drivers/gpu/drm/radeon/radeon_pm.c | 8 ++++++--
>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
>>> b/drivers/gpu/drm/radeon/radeon_pm.c
>>> index 91e1bd2..9f758d3 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_pm.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
>>> @@ -585,7 +585,7 @@ static ssize_t radeon_hwmon_set_pwm1_enable(struct
>>> device *dev,
>>>         if (err)
>>>                 return err;
>>>    -    switch(value) {
>>> +       switch (value) {
>>>         case 1: /* manual, percent-based */
>>>                 rdev->asic->dpm.fan_ctrl_set_mode(rdev,
>>> FDO_PWM_MODE_STATIC);
>>>                 break;
>>> @@ -608,7 +608,7 @@ static ssize_t radeon_hwmon_get_pwm1_max(struct
>>> device *dev,
>>>                                          struct device_attribute *attr,
>>>                                          char *buf)
>>>    {
>>> -       return sprintf(buf, "%i\n", 100); /* pwm uses percent-based
>>> fan-control */
>>> +       return sprintf(buf, "%i\n", 255);
>>>    }
>
>
> This is not a standard name as it is not documented in
> https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface
>
> Apparently, the pwm value should always have been exposed as 0-255 and I
> screwed it up in nouveau by having it be 0-100...
>
> Maybe we should ask pwm*_max to be defined so as new applications could do
> the right thing while not having nouveau change its ABI. I guess it is ok to
> change it for radeon as there are so few users currently but the interface
> has been in nouveau for multiple years already!
>

IIRC, I changed it in the same kernel as the original patch went in.

Alex
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/radeon: use 0-255 rather than 0-100 for pwm fan range
  2015-04-12 23:42     ` Alex Deucher
@ 2015-04-14 21:19       ` Martin Peres
  0 siblings, 0 replies; 5+ messages in thread
From: Martin Peres @ 2015-04-14 21:19 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Maling list - DRI developers

On 13/04/2015 02:42, Alex Deucher wrote:
> On Sat, Apr 11, 2015 at 5:52 AM, Martin Peres <martin.peres@free.fr> wrote:
>> On 05/02/2015 11:49, Christian König wrote:
>>> Am 04.02.2015 um 23:27 schrieb Alex Deucher:
>>>> 0-255 seems to be the preferred range for the pwm interface.
>>>>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> Yeah, using 100 on a 8bit pwm timer sounds rather obviously wrong.
>>>
>>> Patch is Reviewed-by: Christian König <christian.koenig@amd.com>
>>>
>>>> ---
>>>>     drivers/gpu/drm/radeon/radeon_pm.c | 8 ++++++--
>>>>     1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
>>>> b/drivers/gpu/drm/radeon/radeon_pm.c
>>>> index 91e1bd2..9f758d3 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_pm.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
>>>> @@ -585,7 +585,7 @@ static ssize_t radeon_hwmon_set_pwm1_enable(struct
>>>> device *dev,
>>>>          if (err)
>>>>                  return err;
>>>>     -    switch(value) {
>>>> +       switch (value) {
>>>>          case 1: /* manual, percent-based */
>>>>                  rdev->asic->dpm.fan_ctrl_set_mode(rdev,
>>>> FDO_PWM_MODE_STATIC);
>>>>                  break;
>>>> @@ -608,7 +608,7 @@ static ssize_t radeon_hwmon_get_pwm1_max(struct
>>>> device *dev,
>>>>                                           struct device_attribute *attr,
>>>>                                           char *buf)
>>>>     {
>>>> -       return sprintf(buf, "%i\n", 100); /* pwm uses percent-based
>>>> fan-control */
>>>> +       return sprintf(buf, "%i\n", 255);
>>>>     }
>>
>> This is not a standard name as it is not documented in
>> https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface
>>
>> Apparently, the pwm value should always have been exposed as 0-255 and I
>> screwed it up in nouveau by having it be 0-100...
>>
>> Maybe we should ask pwm*_max to be defined so as new applications could do
>> the right thing while not having nouveau change its ABI. I guess it is ok to
>> change it for radeon as there are so few users currently but the interface
>> has been in nouveau for multiple years already!
>>
> IIRC, I changed it in the same kernel as the original patch went in.
>
> Alex
Wonderful!
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-04-14 21:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-04 22:27 [PATCH] drm/radeon: use 0-255 rather than 0-100 for pwm fan range Alex Deucher
2015-02-05  9:49 ` Christian König
2015-04-11  9:52   ` Martin Peres
2015-04-12 23:42     ` Alex Deucher
2015-04-14 21:19       ` Martin Peres

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.