[PATCH] gpu/amd: vega10_hwmgr: fix inappropriate private variable name

Quan, Evan Evan.Quan at amd.com
Fri Feb 25 13:03:54 UTC 2022


[AMD Official Use Only]

Thanks!
The patch is reviewed-by: Evan Quan <evan.quan at amd.com>

> -----Original Message-----
> From: Meng Tang <tangmeng at uniontech.com>
> Sent: Friday, February 25, 2022 5:47 PM
> To: airlied at linux.ie; daniel at ffwll.ch
> Cc: Quan, Evan <Evan.Quan at amd.com>; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Koenig, Christian
> <Christian.Koenig at amd.com>; Pan, Xinhui <Xinhui.Pan at amd.com>; amd-
> gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org; linux-
> kernel at vger.kernel.org; Meng Tang <tangmeng at uniontech.com>
> Subject: [PATCH] gpu/amd: vega10_hwmgr: fix inappropriate private variable
> name
> 
> In file vega10_hwmgr.c, the names of struct vega10_power_state *
> and struct pp_power_state * are confusingly used, which may lead
> to some confusion.
> 
> Status quo is that variables of type struct vega10_power_state *
> are named "vega10_ps", "ps", "vega10_power_state". A more
> appropriate usage is that struct are named "ps" is used for
> variabled of type struct pp_power_state *.
> 
> So rename struct vega10_power_state * which are named "ps" and
> "vega10_power_state" to "vega10_ps", I also renamed "psa" to
> "vega10_psa" and "psb" to "vega10_psb" to make it more clearly.
> 
> The rows longer than 100 columns are involved.
> 
> Signed-off-by: Meng Tang <tangmeng at uniontech.com>
> ---
>  .../drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c | 68 +++++++++++---
> -----
>  1 file changed, 38 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> index 3f040be0d158..37324f2009ca 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_hwmgr.c
> @@ -3095,7 +3095,7 @@ static int
> vega10_get_pp_table_entry_callback_func(struct pp_hwmgr *hwmgr,
>  		void *pp_table, uint32_t classification_flag)
>  {
>  	ATOM_Vega10_GFXCLK_Dependency_Record_V2
> *patom_record_V2;
> -	struct vega10_power_state *vega10_power_state =
> +	struct vega10_power_state *vega10_ps =
>  			cast_phw_vega10_power_state(&(power_state-
> >hardware));
>  	struct vega10_performance_level *performance_level;
>  	ATOM_Vega10_State *state_entry = (ATOM_Vega10_State *)state;
> @@ -3145,17 +3145,17 @@ static int
> vega10_get_pp_table_entry_callback_func(struct pp_hwmgr *hwmgr,
>  	power_state->temperatures.min = 0;
>  	power_state->temperatures.max = 0;
> 
> -	performance_level = &(vega10_power_state->performance_levels
> -			[vega10_power_state-
> >performance_level_count++]);
> +	performance_level = &(vega10_ps->performance_levels
> +			[vega10_ps->performance_level_count++]);
> 
>  	PP_ASSERT_WITH_CODE(
> -			(vega10_power_state->performance_level_count <
> +			(vega10_ps->performance_level_count <
>  					NUM_GFXCLK_DPM_LEVELS),
>  			"Performance levels exceeds SMC limit!",
>  			return -1);
> 
>  	PP_ASSERT_WITH_CODE(
> -			(vega10_power_state->performance_level_count
> <=
> +			(vega10_ps->performance_level_count <=
>  					hwmgr->platform_descriptor.
>  					hardwareActivityPerformanceLevels),
>  			"Performance levels exceeds Driver limit!",
> @@ -3169,8 +3169,8 @@ static int
> vega10_get_pp_table_entry_callback_func(struct pp_hwmgr *hwmgr,
>  	performance_level->mem_clock = mclk_dep_table->entries
>  			[state_entry->ucMemClockIndexLow].ulMemClk;
> 
> -	performance_level = &(vega10_power_state->performance_levels
> -				[vega10_power_state-
> >performance_level_count++]);
> +	performance_level = &(vega10_ps->performance_levels
> +				[vega10_ps->performance_level_count++]);
>  	performance_level->soc_clock = socclk_dep_table->entries
>  				[state_entry->ucSocClockIndexHigh].ulClk;
>  	if (gfxclk_dep_table->ucRevId == 0) {
> @@ -3201,11 +3201,11 @@ static int vega10_get_pp_table_entry(struct
> pp_hwmgr *hwmgr,
>  		unsigned long entry_index, struct pp_power_state *state)
>  {
>  	int result;
> -	struct vega10_power_state *ps;
> +	struct vega10_power_state *vega10_ps;
> 
>  	state->hardware.magic = PhwVega10_Magic;
> 
> -	ps = cast_phw_vega10_power_state(&state->hardware);
> +	vega10_ps = cast_phw_vega10_power_state(&state->hardware);
> 
>  	result = vega10_get_powerplay_table_entry(hwmgr, entry_index,
> state,
>  			vega10_get_pp_table_entry_callback_func);
> @@ -3218,10 +3218,10 @@ static int vega10_get_pp_table_entry(struct
> pp_hwmgr *hwmgr,
>  	 */
>  	/* set DC compatible flag if this state supports DC */
>  	if (!state->validation.disallowOnDC)
> -		ps->dc_compatible = true;
> +		vega10_ps->dc_compatible = true;
> 
> -	ps->uvd_clks.vclk = state->uvd_clocks.VCLK;
> -	ps->uvd_clks.dclk = state->uvd_clocks.DCLK;
> +	vega10_ps->uvd_clks.vclk = state->uvd_clocks.VCLK;
> +	vega10_ps->uvd_clks.dclk = state->uvd_clocks.DCLK;
> 
>  	return 0;
>  }
> @@ -4823,33 +4823,41 @@ static int vega10_check_states_equal(struct
> pp_hwmgr *hwmgr,
>  				const struct pp_hw_power_state *pstate1,
>  			const struct pp_hw_power_state *pstate2, bool
> *equal)
>  {
> -	const struct vega10_power_state *psa;
> -	const struct vega10_power_state *psb;
> +	const struct vega10_power_state *vega10_psa;
> +	const struct vega10_power_state *vega10_psb;
>  	int i;
> 
>  	if (pstate1 == NULL || pstate2 == NULL || equal == NULL)
>  		return -EINVAL;
> 
> -	psa = cast_const_phw_vega10_power_state(pstate1);
> -	psb = cast_const_phw_vega10_power_state(pstate2);
> -	/* If the two states don't even have the same number of
> performance levels they cannot be the same state. */
> -	if (psa->performance_level_count != psb-
> >performance_level_count) {
> +	vega10_psa = cast_const_phw_vega10_power_state(pstate1);
> +	vega10_psb = cast_const_phw_vega10_power_state(pstate2);
> +
> +	/* If the two states don't even have the same number of
> performance levels
> +	 * they cannot be the same state.
> +	 */
> +	if (vega10_psa->performance_level_count != vega10_psb-
> >performance_level_count) {
>  		*equal = false;
>  		return 0;
>  	}
> 
> -	for (i = 0; i < psa->performance_level_count; i++) {
> -		if (!vega10_are_power_levels_equal(&(psa-
> >performance_levels[i]), &(psb->performance_levels[i]))) {
> -			/* If we have found even one performance level pair
> that is different the states are different. */
> +	for (i = 0; i < vega10_psa->performance_level_count; i++) {
> +		if (!vega10_are_power_levels_equal(&(vega10_psa-
> >performance_levels[i]),
> +						   &(vega10_psb-
> >performance_levels[i]))) {
> +			/* If we have found even one performance level pair
> +			 * that is different the states are different.
> +			 */
>  			*equal = false;
>  			return 0;
>  		}
>  	}
> 
>  	/* If all performance levels are the same try to use the UVD clocks to
> break the tie.*/
> -	*equal = ((psa->uvd_clks.vclk == psb->uvd_clks.vclk) && (psa-
> >uvd_clks.dclk == psb->uvd_clks.dclk));
> -	*equal &= ((psa->vce_clks.evclk == psb->vce_clks.evclk) && (psa-
> >vce_clks.ecclk == psb->vce_clks.ecclk));
> -	*equal &= (psa->sclk_threshold == psb->sclk_threshold);
> +	*equal = ((vega10_psa->uvd_clks.vclk == vega10_psb->uvd_clks.vclk)
> &&
> +		  (vega10_psa->uvd_clks.dclk == vega10_psb-
> >uvd_clks.dclk));
> +	*equal &= ((vega10_psa->vce_clks.evclk == vega10_psb-
> >vce_clks.evclk) &&
> +		   (vega10_psa->vce_clks.ecclk == vega10_psb-
> >vce_clks.ecclk));
> +	*equal &= (vega10_psa->sclk_threshold == vega10_psb-
> >sclk_threshold);
> 
>  	return 0;
>  }
> @@ -5444,19 +5452,19 @@ static int vega10_get_performance_level(struct
> pp_hwmgr *hwmgr, const struct pp_
>  				PHM_PerformanceLevelDesignation
> designation, uint32_t index,
>  				PHM_PerformanceLevel *level)
>  {
> -	const struct vega10_power_state *ps;
> +	const struct vega10_power_state *vega10_ps;
>  	uint32_t i;
> 
>  	if (level == NULL || hwmgr == NULL || state == NULL)
>  		return -EINVAL;
> 
> -	ps = cast_const_phw_vega10_power_state(state);
> +	vega10_ps = cast_const_phw_vega10_power_state(state);
> 
> -	i = index > ps->performance_level_count - 1 ?
> -			ps->performance_level_count - 1 : index;
> +	i = index > vega10_ps->performance_level_count - 1 ?
> +			vega10_ps->performance_level_count - 1 : index;
> 
> -	level->coreClock = ps->performance_levels[i].gfx_clock;
> -	level->memory_clock = ps->performance_levels[i].mem_clock;
> +	level->coreClock = vega10_ps->performance_levels[i].gfx_clock;
> +	level->memory_clock = vega10_ps-
> >performance_levels[i].mem_clock;
> 
>  	return 0;
>  }
> --
> 2.20.1
> 
> 


More information about the amd-gfx mailing list