powerplay tidyup/refactor suggestion

Tom St Denis tom.stdenis at amd.com
Tue Oct 3 16:55:18 UTC 2017


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


More information about the amd-gfx mailing list