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