[PATCH] drm/amd/powerplay: fix issue vce dpm can't be enabled on tonga.

Christian König deathsimple at vodafone.de
Tue Aug 30 09:51:13 UTC 2016


Am 30.08.2016 um 11:21 schrieb Rex Zhu:
> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>

One comment below, with that fixed/explained the patch is Reviewed-by: 
Christian König <christian.koenig at amd.com>

>
> Change-Id: If7fa45cf3e81c31a8828f208ee9ba02bb9f18a1c
> ---
>   .../amd/powerplay/hwmgr/tonga_clockpowergating.c   | 60 +++++-----------------
>   drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c  | 34 ++++++------
>   drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.h  |  2 +-
>   3 files changed, 31 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_clockpowergating.c b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_clockpowergating.c
> index e58d038..6f2ac10 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_clockpowergating.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_clockpowergating.c
> @@ -137,55 +137,23 @@ int tonga_phm_powergate_uvd(struct pp_hwmgr *hwmgr, bool bgate)
>   int tonga_phm_powergate_vce(struct pp_hwmgr *hwmgr, bool bgate)
>   {
>   	struct tonga_hwmgr *data = (struct tonga_hwmgr *)(hwmgr->backend);
> -	struct phm_set_power_state_input states;
> -	const struct pp_power_state  *pcurrent;
> -	struct pp_power_state  *requested;
> -
> -	pcurrent = hwmgr->current_ps;
> -	requested = hwmgr->request_ps;
> -
> -	states.pcurrent_state = &(pcurrent->hardware);
> -	states.pnew_state = &(requested->hardware);
> -
> -	if (phm_cf_want_vce_power_gating(hwmgr)) {
> -		if (data->vce_power_gated != bgate) {
> -			if (bgate) {
> -				cgs_set_clockgating_state(
> -							hwmgr->device,
> -							AMD_IP_BLOCK_TYPE_VCE,
> -							AMD_CG_STATE_UNGATE);
> -				cgs_set_powergating_state(
> -							hwmgr->device,
> -							AMD_IP_BLOCK_TYPE_VCE,
> -							AMD_PG_STATE_GATE);
> -				tonga_enable_disable_vce_dpm(hwmgr, false);
> -				data->vce_power_gated = true;
> -			} else {
> -				tonga_phm_powerup_vce(hwmgr);
> -				data->vce_power_gated = false;
> -				cgs_set_powergating_state(
> -							hwmgr->device,
> -							AMD_IP_BLOCK_TYPE_VCE,
> -							AMD_PG_STATE_UNGATE);
> -				cgs_set_clockgating_state(
> -							hwmgr->device,
> -							AMD_IP_BLOCK_TYPE_VCE,
> -							AMD_PG_STATE_GATE);
> -
> -				tonga_update_vce_dpm(hwmgr, &states);
> -				tonga_enable_disable_vce_dpm(hwmgr, true);
> -				return 0;
> -			}
> -		}
> -	} else {
> -		tonga_update_vce_dpm(hwmgr, &states);
> -		tonga_enable_disable_vce_dpm(hwmgr, true);
> +
> +	if (data->vce_power_gated == bgate)
>   		return 0;
> -	}
>   
> -	if (!data->vce_power_gated)
> -		tonga_update_vce_dpm(hwmgr, &states);
> +	data->vce_power_gated = bgate;
>   
> +	if (bgate) {
> +		cgs_set_clockgating_state(hwmgr->device,
> +				AMD_IP_BLOCK_TYPE_VCE,
> +				AMD_CG_STATE_GATE);
> +		tonga_update_vce_dpm(hwmgr, true);
> +	} else {
> +		tonga_update_vce_dpm(hwmgr, false);
> +		cgs_set_clockgating_state(hwmgr->device,
> +				AMD_IP_BLOCK_TYPE_VCE,
> +				AMD_CG_STATE_UNGATE);
> +	}
>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
> index e09847d..2c1c4f8 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
> @@ -5567,35 +5567,36 @@ int tonga_update_uvd_dpm(struct pp_hwmgr *hwmgr, bool bgate)
>   	return tonga_enable_disable_uvd_dpm(hwmgr, !bgate);
>   }
>   
> -int tonga_update_vce_dpm(struct pp_hwmgr *hwmgr, const void *input)
> +int tonga_update_vce_dpm(struct pp_hwmgr *hwmgr, bool bgate)
>   {
> -	const struct phm_set_power_state_input *states = (const struct phm_set_power_state_input *)input;
>   	struct tonga_hwmgr *data = (struct tonga_hwmgr *)(hwmgr->backend);
> -	const struct tonga_power_state *tonga_nps = cast_const_phw_tonga_power_state(states->pnew_state);
> -	const struct tonga_power_state *tonga_cps = cast_const_phw_tonga_power_state(states->pcurrent_state);
> -
>   	uint32_t mm_boot_level_offset, mm_boot_level_value;
> -	struct phm_ppt_v1_information *pptable_info = (struct phm_ppt_v1_information *)(hwmgr->pptable);
> +	struct phm_ppt_v1_information *table_info = (struct phm_ppt_v1_information *)(hwmgr->pptable);
>   
> -	if (tonga_nps->vce_clocks.EVCLK > 0 && (tonga_cps == NULL || tonga_cps->vce_clocks.EVCLK == 0)) {
> -		data->smc_state_table.VceBootLevel = (uint8_t) (pptable_info->mm_dep_table->count - 1);
> +	if (!bgate) {
> +		if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
> +						PHM_PlatformCaps_StablePState))
> +			data->smc_state_table.VceBootLevel =
> +				(uint8_t) (table_info->mm_dep_table->count - 1);
>   
> -		mm_boot_level_offset = data->dpm_table_start + offsetof(SMU72_Discrete_DpmTable, VceBootLevel);
> +		mm_boot_level_offset = data->dpm_table_start +
> +				offsetof(SMU72_Discrete_DpmTable, VceBootLevel);

>   		mm_boot_level_offset /= 4;
>   		mm_boot_level_offset *= 4;

This looks odd. Is that just to mask out the lower bits or what is that 
all about?

If it's just for masking then I would suggest to use 
"mm_boot_level_offset &= 3" instead.

If it's for alignment then please use the ALIGN() macro.

If it's something totally else then please add a comment explaining what 
we do here.

Regards,
Christian.


> -		mm_boot_level_value = cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, mm_boot_level_offset);
> +		mm_boot_level_value = cgs_read_ind_register(hwmgr->device,
> +				CGS_IND_REG__SMC, mm_boot_level_offset);
>   		mm_boot_level_value &= 0xFF00FFFF;
>   		mm_boot_level_value |= data->smc_state_table.VceBootLevel << 16;
> -		cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, mm_boot_level_offset, mm_boot_level_value);
> +		cgs_write_ind_register(hwmgr->device,
> +				CGS_IND_REG__SMC, mm_boot_level_offset, mm_boot_level_value);
>   
>   		if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps, PHM_PlatformCaps_StablePState))
>   			smum_send_msg_to_smc_with_parameter(hwmgr->smumgr,
>   					PPSMC_MSG_VCEDPM_SetEnabledMask,
> -				(uint32_t)(1 << data->smc_state_table.VceBootLevel));
> +					(uint32_t)1 << data->smc_state_table.VceBootLevel);
> +	}
>   
> -		tonga_enable_disable_vce_dpm(hwmgr, true);
> -	} else if (tonga_nps->vce_clocks.EVCLK == 0 && tonga_cps != NULL && tonga_cps->vce_clocks.EVCLK > 0)
> -		tonga_enable_disable_vce_dpm(hwmgr, false);
> +	tonga_enable_disable_vce_dpm(hwmgr, !bgate);
>   
>   	return 0;
>   }
> @@ -5732,9 +5733,6 @@ static int tonga_set_power_state_tasks(struct pp_hwmgr *hwmgr, const void *input
>   	tmp_result = tonga_generate_dpm_level_enable_mask(hwmgr, input);
>   	PP_ASSERT_WITH_CODE((0 == tmp_result), "Failed to generate DPM level enabled mask!", result = tmp_result);
>   
> -	tmp_result = tonga_update_vce_dpm(hwmgr, input);
> -	PP_ASSERT_WITH_CODE((0 == tmp_result), "Failed to update VCE DPM!", result = tmp_result);
> -
>   	tmp_result = tonga_update_sclk_threshold(hwmgr);
>   	PP_ASSERT_WITH_CODE((0 == tmp_result), "Failed to update SCLK threshold!", result = tmp_result);
>   
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.h b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.h
> index fcad942..f6f76d4 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.h
> @@ -392,7 +392,7 @@ typedef struct tonga_hwmgr tonga_hwmgr;
>   #define TONGA_UNUSED_GPIO_PIN                        0x7F
>   
>   int tonga_hwmgr_init(struct pp_hwmgr *hwmgr);
> -int tonga_update_vce_dpm(struct pp_hwmgr *hwmgr, const void *input);
> +int tonga_update_vce_dpm(struct pp_hwmgr *hwmgr, bool bgate);
>   int tonga_update_uvd_dpm(struct pp_hwmgr *hwmgr, bool bgate);
>   int tonga_enable_disable_uvd_dpm(struct pp_hwmgr *hwmgr, bool enable);
>   int tonga_enable_disable_vce_dpm(struct pp_hwmgr *hwmgr, bool enable);




More information about the amd-gfx mailing list