[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 at 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 at 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);
>>
>