Re: [PATCH V2 4/5] drm/amd/pm: clear the cached dpm feature status

["Lazar, Lijo" <lijo.lazar@xxxxxxx>]

On 6/4/2021 3:28 PM, Evan Quan wrote:
> For some ASICs, the real dpm feature disablement job is handled by
> PMFW during baco reset and custom pptable loading. Cached dpm feature
> status need to be cleared to pair that.
>
> Change-Id: I9e37d80e13599833301c04711b097fb37c2e41f9
> Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
> ---
> V1->V2:
>    - correct the setting for baco bit(pointed out by Lijo)
> ---
>   drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h       |  6 +++++
>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 19 ++++++++++++--
>   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   |  1 +
>   .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   |  1 +
>   drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c        | 25 +++++++++++++++++++
>   drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h        |  3 +++
>   drivers/gpu/drm/amd/pm/swsmu/smu_internal.h   |  1 +
>   7 files changed, 54 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> index fb7438224872..3f9b2a839a37 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> @@ -937,6 +937,12 @@ struct pptable_funcs {
>   	 */
>   	int (*disable_all_features_with_exception)(struct smu_context *smu, enum smu_feature_mask mask);
>   
> +	/**
> +	 * @clear_cached_dpm_feature_status_with_exception: Clear cached dpm feature status with
> +	 *                                                  exception to those in &mask.
> +	 */
> +	int (*clear_cached_dpm_feature_status_with_exception)(struct smu_context *smu, enum smu_feature_mask mask);
> +

Since this is a just a utility function which clears sw bitmask without 
touching hardware, do we need a ppt_func? In case that is needed, we 
already have this - smu_cmn_disable_all_features_with_exception. So, 
can't this remain as a standalone smu_cmn function which can be called 
from anywhere? Nowadays, I get a red alert when a ppt_func gets added :)

Thanks,
Lijo

>   	/**
>   	 * @notify_display_change: Enable fast memory clock switching.
>   	 *
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index c9a31556e077..aba475d06507 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1352,6 +1352,16 @@ static int smu_hw_init(void *handle)
>   	return 0;
>   }
>   
> +static void smu_reset_cached_dpm_feature_status(struct smu_context *smu)
> +{
> +	struct smu_feature *feature = &smu->smu_feature;
> +
> +	mutex_lock(&feature->mutex);
> +	bitmap_zero(feature->enabled, feature->feature_num);
> +	bitmap_zero(feature->supported, feature->feature_num);
> +	mutex_unlock(&feature->mutex);
> +}
> +
>   static int smu_disable_dpms(struct smu_context *smu)
>   {
>   	struct amdgpu_device *adev = smu->adev;
> @@ -1374,16 +1384,21 @@ static int smu_disable_dpms(struct smu_context *smu)
>   	 */
>   	if (smu->uploading_custom_pp_table &&
>   	    (adev->asic_type >= CHIP_NAVI10) &&
> -	    (adev->asic_type <= CHIP_DIMGREY_CAVEFISH))
> +	    (adev->asic_type <= CHIP_DIMGREY_CAVEFISH)) {
> +		smu_reset_cached_dpm_feature_status(smu);
>   		return 0;
> +	}
>   
>   	/*
>   	 * For Sienna_Cichlid, PMFW will handle the features disablement properly
>   	 * on BACO in. Driver involvement is unnecessary.
>   	 */
>   	if ((adev->asic_type == CHIP_SIENNA_CICHLID) &&
> -	     use_baco)
> +	     use_baco) {
> +		smu_clear_cached_dpm_feature_status_with_exception(smu,
> +								   SMU_FEATURE_BACO_BIT);
>   		return 0;
> +	}
>   
>   	/*
>   	 * For gpu reset, runpm and hibernation through BACO,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index afd4bd850c63..8c3ea20fb0f6 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -3083,6 +3083,7 @@ static const struct pptable_funcs navi10_ppt_funcs = {
>   	.get_enabled_mask = smu_cmn_get_enabled_mask,
>   	.feature_is_enabled = smu_cmn_feature_is_enabled,
>   	.disable_all_features_with_exception = smu_cmn_disable_all_features_with_exception,
> +	.clear_cached_dpm_feature_status_with_exception = smu_cmn_clear_cached_dpm_feature_status_with_exception,
>   	.notify_display_change = smu_v11_0_notify_display_change,
>   	.set_power_limit = smu_v11_0_set_power_limit,
>   	.init_max_sustainable_clocks = smu_v11_0_init_max_sustainable_clocks,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> index b4a2bb0f3f4c..14a14c30ee5e 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> @@ -3886,6 +3886,7 @@ static const struct pptable_funcs sienna_cichlid_ppt_funcs = {
>   	.get_enabled_mask = smu_cmn_get_enabled_mask,
>   	.feature_is_enabled = smu_cmn_feature_is_enabled,
>   	.disable_all_features_with_exception = smu_cmn_disable_all_features_with_exception,
> +	.clear_cached_dpm_feature_status_with_exception = smu_cmn_clear_cached_dpm_feature_status_with_exception,
>   	.notify_display_change = NULL,
>   	.set_power_limit = smu_v11_0_set_power_limit,
>   	.init_max_sustainable_clocks = smu_v11_0_init_max_sustainable_clocks,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index 0ceb7329838c..b9c38abc9d37 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -594,6 +594,31 @@ int smu_cmn_disable_all_features_with_exception(struct smu_context *smu,
>   						   0);
>   }
>   
> +int smu_cmn_clear_cached_dpm_feature_status_with_exception(struct smu_context *smu,
> +							   enum smu_feature_mask mask)
> +{
> +	struct smu_feature *feature = &smu->smu_feature;
> +	uint64_t features_to_disable = U64_MAX;
> +	int skipped_feature_id;
> +
> +	skipped_feature_id = smu_cmn_to_asic_specific_index(smu,
> +							    CMN2ASIC_MAPPING_FEATURE,
> +							    mask);
> +	if (skipped_feature_id < 0)
> +		return -EINVAL;
> +
> +	features_to_disable &= ~(1ULL << skipped_feature_id);
> +
> +	mutex_lock(&feature->mutex);
> +	bitmap_andnot(feature->enabled, feature->enabled,
> +			(unsigned long *)(&features_to_disable), SMU_FEATURE_MAX);
> +	bitmap_andnot(feature->supported, feature->supported,
> +			(unsigned long *)(&features_to_disable), SMU_FEATURE_MAX);
> +	mutex_unlock(&feature->mutex);
> +
> +	return 0;
> +}
> +
>   int smu_cmn_get_smc_version(struct smu_context *smu,
>   			    uint32_t *if_version,
>   			    uint32_t *smu_version)
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> index da6ff6f024f9..12ad07132d15 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> @@ -77,6 +77,9 @@ int smu_cmn_set_pp_feature_mask(struct smu_context *smu,
>   int smu_cmn_disable_all_features_with_exception(struct smu_context *smu,
>   						enum smu_feature_mask mask);
>   
> +int smu_cmn_clear_cached_dpm_feature_status_with_exception(struct smu_context *smu,
> +							   enum smu_feature_mask mask);
> +
>   int smu_cmn_get_smc_version(struct smu_context *smu,
>   			    uint32_t *if_version,
>   			    uint32_t *smu_version);
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> index 33101dc93bcc..b12dcfab55fe 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> @@ -58,6 +58,7 @@
>   #define smu_feature_get_enabled_mask(smu, mask, num)			smu_ppt_funcs(get_enabled_mask, 0, smu, mask, num)
>   #define smu_feature_is_enabled(smu, mask)				smu_ppt_funcs(feature_is_enabled, 0, smu, mask)
>   #define smu_disable_all_features_with_exception(smu, mask)		smu_ppt_funcs(disable_all_features_with_exception, 0, smu, mask)
> +#define smu_clear_cached_dpm_feature_status_with_exception(smu, mask)	smu_ppt_funcs(clear_cached_dpm_feature_status_with_exception, 0, smu, mask)
>   #define smu_is_dpm_running(smu)						smu_ppt_funcs(is_dpm_running, 0 , smu)
>   #define smu_notify_display_change(smu)					smu_ppt_funcs(notify_display_change, 0, smu)
>   #define smu_populate_umd_state_clk(smu)					smu_ppt_funcs(populate_umd_state_clk, 0, smu)
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx