Hi all,
Soliciting for opinion on a tiny refactor I've noticed is possible in
the hwmgr API, we have functions like
int (*dynamic_state_management_enable)(
struct pp_hwmgr *hw_mgr);
int (*dynamic_state_management_disable)(
struct pp_hwmgr *hw_mgr);
Which could be merged with a 2nd parameter, or:
uint32_t (*get_mclk)(struct pp_hwmgr *hwmgr, bool low);
uint32_t (*get_sclk)(struct pp_hwmgr *hwmgr, bool low);
Could be merged as well (with a 3rd parameter), and:
int (*get_sclk_od)(struct pp_hwmgr *hwmgr);
int (*set_sclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
int (*get_mclk_od)(struct pp_hwmgr *hwmgr);
int (*set_mclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
Could be merged somewhat, etc.
The gain is that we have duplicate blocks like:
static uint32_t smu7_dpm_get_mclk(struct pp_hwmgr *hwmgr, bool low)
{
struct pp_power_state *ps;
struct smu7_power_state *smu7_ps;
if (hwmgr == NULL)
return -EINVAL;
ps = hwmgr->request_ps;
if (ps == NULL)
return -EINVAL;
smu7_ps = cast_phw_smu7_power_state(&ps->hardware);
if (low)
return smu7_ps->performance_levels[0].memory_clock;
else
return smu7_ps->performance_levels
[smu7_ps->performance_level_count-1].memory_clock;
}
static uint32_t smu7_dpm_get_sclk(struct pp_hwmgr *hwmgr, bool low)
{
struct pp_power_state *ps;
struct smu7_power_state *smu7_ps;
if (hwmgr == NULL)
return -EINVAL;
ps = hwmgr->request_ps;
if (ps == NULL)
return -EINVAL;
smu7_ps = cast_phw_smu7_power_state(&ps->hardware);
if (low)
return smu7_ps->performance_levels[0].engine_clock;
else
return smu7_ps->performance_levels
[smu7_ps->performance_level_count-1].engine_clock;
}
Where most of the two functions are the same and could be reduced.
Anyways nothing earth shattering but would help reduce some LOC and make
the API simpler. I could tackle this for the various hwmgr's all in one
series.
Thoughts?
Tom
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
