drm/amd/powerplay: update cached feature enablement status V3

Submitted by Quan, Evan on Aug. 23, 2019, 4:48 a.m.

Details

Message ID 20190823044851.8133-1-evan.quan@amd.com
State New
Headers show
Series "drm/amd/powerplay: update cached feature enablement status" ( rev: 4 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Quan, Evan Aug. 23, 2019, 4:48 a.m.
Need to update in cache feature enablement status after pp_feature
settings. Another fix for the commit below:
drm/amd/powerplay: implment sysfs feature status function in smu

V2: update smu_feature_update_enable_state() and relates
V3: use bitmap_or and bitmap_andnot

Change-Id: I90e29b0d839df26825d5993212f6097c7ad4bebf
Signed-off-by: Evan Quan <evan.quan@amd.com>
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c    | 101 +++++++++---------
 .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h    |   1 -
 2 files changed, 49 insertions(+), 53 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 4df7fb6eaf3c..c8c00966a621 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -94,6 +94,52 @@  size_t smu_sys_get_pp_feature_mask(struct smu_context *smu, char *buf)
 	return size;
 }
 
+static int smu_feature_update_enable_state(struct smu_context *smu,
+					   uint64_t feature_mask,
+					   bool enabled)
+{
+	struct smu_feature *feature = &smu->smu_feature;
+	uint32_t feature_low = 0, feature_high = 0;
+	int ret = 0;
+
+	if (!smu->pm_enabled)
+		return ret;
+
+	feature_low = (feature_mask >> 0 ) & 0xffffffff;
+	feature_high = (feature_mask >> 32) & 0xffffffff;
+
+	if (enabled) {
+		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesLow,
+						  feature_low);
+		if (ret)
+			return ret;
+		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesHigh,
+						  feature_high);
+		if (ret)
+			return ret;
+	} else {
+		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesLow,
+						  feature_low);
+		if (ret)
+			return ret;
+		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesHigh,
+						  feature_high);
+		if (ret)
+			return ret;
+	}
+
+	mutex_lock(&feature->mutex);
+	if (enabled)
+		bitmap_or(feature->enabled, feature->enabled,
+				(unsigned long *)(&feature_mask), SMU_FEATURE_MAX);
+	else
+		bitmap_andnot(feature->enabled, feature->enabled,
+				(unsigned long *)(&feature_mask), SMU_FEATURE_MAX);
+	mutex_unlock(&feature->mutex);
+
+	return ret;
+}
+
 int smu_sys_set_pp_feature_mask(struct smu_context *smu, uint64_t new_mask)
 {
 	int ret = 0;
@@ -591,41 +637,7 @@  int smu_feature_init_dpm(struct smu_context *smu)
 
 	return ret;
 }
-int smu_feature_update_enable_state(struct smu_context *smu, uint64_t feature_mask, bool enabled)
-{
-	uint32_t feature_low = 0, feature_high = 0;
-	int ret = 0;
-
-	if (!smu->pm_enabled)
-		return ret;
-
-	feature_low = (feature_mask >> 0 ) & 0xffffffff;
-	feature_high = (feature_mask >> 32) & 0xffffffff;
-
-	if (enabled) {
-		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesLow,
-						  feature_low);
-		if (ret)
-			return ret;
-		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesHigh,
-						  feature_high);
-		if (ret)
-			return ret;
-
-	} else {
-		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesLow,
-						  feature_low);
-		if (ret)
-			return ret;
-		ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesHigh,
-						  feature_high);
-		if (ret)
-			return ret;
 
-	}
-
-	return ret;
-}
 
 int smu_feature_is_enabled(struct smu_context *smu, enum smu_feature_mask mask)
 {
@@ -651,8 +663,6 @@  int smu_feature_set_enabled(struct smu_context *smu, enum smu_feature_mask mask,
 {
 	struct smu_feature *feature = &smu->smu_feature;
 	int feature_id;
-	uint64_t feature_mask = 0;
-	int ret = 0;
 
 	feature_id = smu_feature_get_index(smu, mask);
 	if (feature_id < 0)
@@ -660,22 +670,9 @@  int smu_feature_set_enabled(struct smu_context *smu, enum smu_feature_mask mask,
 
 	WARN_ON(feature_id > feature->feature_num);
 
-	feature_mask = 1ULL << feature_id;
-
-	mutex_lock(&feature->mutex);
-	ret = smu_feature_update_enable_state(smu, feature_mask, enable);
-	if (ret)
-		goto failed;
-
-	if (enable)
-		test_and_set_bit(feature_id, feature->enabled);
-	else
-		test_and_clear_bit(feature_id, feature->enabled);
-
-failed:
-	mutex_unlock(&feature->mutex);
-
-	return ret;
+	return smu_feature_update_enable_state(smu,
+					       1ULL << feature_id,
+					       enable);
 }
 
 int smu_feature_is_supported(struct smu_context *smu, enum smu_feature_mask mask)
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index e80c81552d29..fbf68fd42b93 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -807,7 +807,6 @@  enum amd_dpm_forced_level smu_get_performance_level(struct smu_context *smu);
 int smu_force_performance_level(struct smu_context *smu, enum amd_dpm_forced_level level);
 int smu_set_display_count(struct smu_context *smu, uint32_t count);
 bool smu_clk_dpm_is_enabled(struct smu_context *smu, enum smu_clk_type clk_type);
-int smu_feature_update_enable_state(struct smu_context *smu, uint64_t feature_mask, bool enabled);
 const char *smu_get_message_name(struct smu_context *smu, enum smu_message_type type);
 const char *smu_get_feature_name(struct smu_context *smu, enum smu_feature_mask feature);
 size_t smu_sys_get_pp_feature_mask(struct smu_context *smu, char *buf);

Comments

Ping..

> -----Original Message-----

> From: Evan Quan <evan.quan@amd.com>

> Sent: 2019年8月23日 12:49

> To: amd-gfx@lists.freedesktop.org

> Cc: Quan, Evan <Evan.Quan@amd.com>

> Subject: [PATCH] drm/amd/powerplay: update cached feature enablement

> status V3

> 

> Need to update in cache feature enablement status after pp_feature

> settings. Another fix for the commit below:

> drm/amd/powerplay: implment sysfs feature status function in smu

> 

> V2: update smu_feature_update_enable_state() and relates

> V3: use bitmap_or and bitmap_andnot

> 

> Change-Id: I90e29b0d839df26825d5993212f6097c7ad4bebf

> Signed-off-by: Evan Quan <evan.quan@amd.com>

> ---

>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c    | 101 +++++++++--------

> -

>  .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h    |   1 -

>  2 files changed, 49 insertions(+), 53 deletions(-)

> 

> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> index 4df7fb6eaf3c..c8c00966a621 100644

> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c

> @@ -94,6 +94,52 @@ size_t smu_sys_get_pp_feature_mask(struct

> smu_context *smu, char *buf)

>  	return size;

>  }

> 

> +static int smu_feature_update_enable_state(struct smu_context *smu,

> +					   uint64_t feature_mask,

> +					   bool enabled)

> +{

> +	struct smu_feature *feature = &smu->smu_feature;

> +	uint32_t feature_low = 0, feature_high = 0;

> +	int ret = 0;

> +

> +	if (!smu->pm_enabled)

> +		return ret;

> +

> +	feature_low = (feature_mask >> 0 ) & 0xffffffff;

> +	feature_high = (feature_mask >> 32) & 0xffffffff;

> +

> +	if (enabled) {

> +		ret = smu_send_smc_msg_with_param(smu,

> SMU_MSG_EnableSmuFeaturesLow,

> +						  feature_low);

> +		if (ret)

> +			return ret;

> +		ret = smu_send_smc_msg_with_param(smu,

> SMU_MSG_EnableSmuFeaturesHigh,

> +						  feature_high);

> +		if (ret)

> +			return ret;

> +	} else {

> +		ret = smu_send_smc_msg_with_param(smu,

> SMU_MSG_DisableSmuFeaturesLow,

> +						  feature_low);

> +		if (ret)

> +			return ret;

> +		ret = smu_send_smc_msg_with_param(smu,

> SMU_MSG_DisableSmuFeaturesHigh,

> +						  feature_high);

> +		if (ret)

> +			return ret;

> +	}

> +

> +	mutex_lock(&feature->mutex);

> +	if (enabled)

> +		bitmap_or(feature->enabled, feature->enabled,

> +				(unsigned long *)(&feature_mask),

> SMU_FEATURE_MAX);

> +	else

> +		bitmap_andnot(feature->enabled, feature->enabled,

> +				(unsigned long *)(&feature_mask),

> SMU_FEATURE_MAX);

> +	mutex_unlock(&feature->mutex);

> +

> +	return ret;

> +}

> +

>  int smu_sys_set_pp_feature_mask(struct smu_context *smu, uint64_t

> new_mask)  {

>  	int ret = 0;

> @@ -591,41 +637,7 @@ int smu_feature_init_dpm(struct smu_context

> *smu)

> 

>  	return ret;

>  }

> -int smu_feature_update_enable_state(struct smu_context *smu, uint64_t

> feature_mask, bool enabled) -{

> -	uint32_t feature_low = 0, feature_high = 0;

> -	int ret = 0;

> -

> -	if (!smu->pm_enabled)

> -		return ret;

> -

> -	feature_low = (feature_mask >> 0 ) & 0xffffffff;

> -	feature_high = (feature_mask >> 32) & 0xffffffff;

> -

> -	if (enabled) {

> -		ret = smu_send_smc_msg_with_param(smu,

> SMU_MSG_EnableSmuFeaturesLow,

> -						  feature_low);

> -		if (ret)

> -			return ret;

> -		ret = smu_send_smc_msg_with_param(smu,

> SMU_MSG_EnableSmuFeaturesHigh,

> -						  feature_high);

> -		if (ret)

> -			return ret;

> -

> -	} else {

> -		ret = smu_send_smc_msg_with_param(smu,

> SMU_MSG_DisableSmuFeaturesLow,

> -						  feature_low);

> -		if (ret)

> -			return ret;

> -		ret = smu_send_smc_msg_with_param(smu,

> SMU_MSG_DisableSmuFeaturesHigh,

> -						  feature_high);

> -		if (ret)

> -			return ret;

> 

> -	}

> -

> -	return ret;

> -}

> 

>  int smu_feature_is_enabled(struct smu_context *smu, enum

> smu_feature_mask mask)  { @@ -651,8 +663,6 @@ int

> smu_feature_set_enabled(struct smu_context *smu, enum

> smu_feature_mask mask,  {

>  	struct smu_feature *feature = &smu->smu_feature;

>  	int feature_id;

> -	uint64_t feature_mask = 0;

> -	int ret = 0;

> 

>  	feature_id = smu_feature_get_index(smu, mask);

>  	if (feature_id < 0)

> @@ -660,22 +670,9 @@ int smu_feature_set_enabled(struct smu_context

> *smu, enum smu_feature_mask mask,

> 

>  	WARN_ON(feature_id > feature->feature_num);

> 

> -	feature_mask = 1ULL << feature_id;

> -

> -	mutex_lock(&feature->mutex);

> -	ret = smu_feature_update_enable_state(smu, feature_mask,

> enable);

> -	if (ret)

> -		goto failed;

> -

> -	if (enable)

> -		test_and_set_bit(feature_id, feature->enabled);

> -	else

> -		test_and_clear_bit(feature_id, feature->enabled);

> -

> -failed:

> -	mutex_unlock(&feature->mutex);

> -

> -	return ret;

> +	return smu_feature_update_enable_state(smu,

> +					       1ULL << feature_id,

> +					       enable);

>  }

> 

>  int smu_feature_is_supported(struct smu_context *smu, enum

> smu_feature_mask mask) diff --git

> a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> index e80c81552d29..fbf68fd42b93 100644

> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h

> @@ -807,7 +807,6 @@ enum amd_dpm_forced_level

> smu_get_performance_level(struct smu_context *smu);  int

> smu_force_performance_level(struct smu_context *smu, enum

> amd_dpm_forced_level level);  int smu_set_display_count(struct

> smu_context *smu, uint32_t count);  bool smu_clk_dpm_is_enabled(struct

> smu_context *smu, enum smu_clk_type clk_type); -int

> smu_feature_update_enable_state(struct smu_context *smu, uint64_t

> feature_mask, bool enabled);  const char *smu_get_message_name(struct

> smu_context *smu, enum smu_message_type type);  const char

> *smu_get_feature_name(struct smu_context *smu, enum

> smu_feature_mask feature);  size_t smu_sys_get_pp_feature_mask(struct

> smu_context *smu, char *buf);

> --

> 2.23.0
Reviewed-by: Jack Gui <Jack.Gui@amd.com>

-----Original Message-----
From: Quan, Evan <Evan.Quan@amd.com> 
Sent: Monday, September 2, 2019 4:16 PM
To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Gui, Jack <Jack.Gui@amd.com>
Subject: RE: [PATCH] drm/amd/powerplay: update cached feature enablement status V3

Ping..

> -----Original Message-----
> From: Evan Quan <evan.quan@amd.com>
> Sent: 2019年8月23日 12:49
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan <Evan.Quan@amd.com>
> Subject: [PATCH] drm/amd/powerplay: update cached feature enablement 
> status V3
> 
> Need to update in cache feature enablement status after pp_feature 
> settings. Another fix for the commit below:
> drm/amd/powerplay: implment sysfs feature status function in smu
> 
> V2: update smu_feature_update_enable_state() and relates
> V3: use bitmap_or and bitmap_andnot
> 
> Change-Id: I90e29b0d839df26825d5993212f6097c7ad4bebf
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> ---
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c    | 101 +++++++++--------
> -
>  .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h    |   1 -
>  2 files changed, 49 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 4df7fb6eaf3c..c8c00966a621 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -94,6 +94,52 @@ size_t smu_sys_get_pp_feature_mask(struct
> smu_context *smu, char *buf)
>  	return size;
>  }
> 
> +static int smu_feature_update_enable_state(struct smu_context *smu,
> +					   uint64_t feature_mask,
> +					   bool enabled)
> +{
> +	struct smu_feature *feature = &smu->smu_feature;
> +	uint32_t feature_low = 0, feature_high = 0;
> +	int ret = 0;
> +
> +	if (!smu->pm_enabled)
> +		return ret;
> +
> +	feature_low = (feature_mask >> 0 ) & 0xffffffff;
> +	feature_high = (feature_mask >> 32) & 0xffffffff;
> +
> +	if (enabled) {
> +		ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_EnableSmuFeaturesLow,
> +						  feature_low);
> +		if (ret)
> +			return ret;
> +		ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_EnableSmuFeaturesHigh,
> +						  feature_high);
> +		if (ret)
> +			return ret;
> +	} else {
> +		ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_DisableSmuFeaturesLow,
> +						  feature_low);
> +		if (ret)
> +			return ret;
> +		ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_DisableSmuFeaturesHigh,
> +						  feature_high);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	mutex_lock(&feature->mutex);
> +	if (enabled)
> +		bitmap_or(feature->enabled, feature->enabled,
> +				(unsigned long *)(&feature_mask),
> SMU_FEATURE_MAX);
> +	else
> +		bitmap_andnot(feature->enabled, feature->enabled,
> +				(unsigned long *)(&feature_mask),
> SMU_FEATURE_MAX);
> +	mutex_unlock(&feature->mutex);
> +
> +	return ret;
> +}
> +
>  int smu_sys_set_pp_feature_mask(struct smu_context *smu, uint64_t
> new_mask)  {
>  	int ret = 0;
> @@ -591,41 +637,7 @@ int smu_feature_init_dpm(struct smu_context
> *smu)
> 
>  	return ret;
>  }
> -int smu_feature_update_enable_state(struct smu_context *smu, uint64_t 
> feature_mask, bool enabled) -{
> -	uint32_t feature_low = 0, feature_high = 0;
> -	int ret = 0;
> -
> -	if (!smu->pm_enabled)
> -		return ret;
> -
> -	feature_low = (feature_mask >> 0 ) & 0xffffffff;
> -	feature_high = (feature_mask >> 32) & 0xffffffff;
> -
> -	if (enabled) {
> -		ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_EnableSmuFeaturesLow,
> -						  feature_low);
> -		if (ret)
> -			return ret;
> -		ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_EnableSmuFeaturesHigh,
> -						  feature_high);
> -		if (ret)
> -			return ret;
> -
> -	} else {
> -		ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_DisableSmuFeaturesLow,
> -						  feature_low);
> -		if (ret)
> -			return ret;
> -		ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_DisableSmuFeaturesHigh,
> -						  feature_high);
> -		if (ret)
> -			return ret;
> 
> -	}
> -
> -	return ret;
> -}
> 
>  int smu_feature_is_enabled(struct smu_context *smu, enum 
> smu_feature_mask mask)  { @@ -651,8 +663,6 @@ int 
> smu_feature_set_enabled(struct smu_context *smu, enum smu_feature_mask 
> mask,  {
>  	struct smu_feature *feature = &smu->smu_feature;
>  	int feature_id;
> -	uint64_t feature_mask = 0;
> -	int ret = 0;
> 
>  	feature_id = smu_feature_get_index(smu, mask);
>  	if (feature_id < 0)
> @@ -660,22 +670,9 @@ int smu_feature_set_enabled(struct smu_context 
> *smu, enum smu_feature_mask mask,
> 
>  	WARN_ON(feature_id > feature->feature_num);
> 
> -	feature_mask = 1ULL << feature_id;
> -
> -	mutex_lock(&feature->mutex);
> -	ret = smu_feature_update_enable_state(smu, feature_mask,
> enable);
> -	if (ret)
> -		goto failed;
> -
> -	if (enable)
> -		test_and_set_bit(feature_id, feature->enabled);
> -	else
> -		test_and_clear_bit(feature_id, feature->enabled);
> -
> -failed:
> -	mutex_unlock(&feature->mutex);
> -
> -	return ret;
> +	return smu_feature_update_enable_state(smu,
> +					       1ULL << feature_id,
> +					       enable);
>  }
> 
>  int smu_feature_is_supported(struct smu_context *smu, enum 
> smu_feature_mask mask) diff --git 
> a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index e80c81552d29..fbf68fd42b93 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -807,7 +807,6 @@ enum amd_dpm_forced_level 
> smu_get_performance_level(struct smu_context *smu);  int 
> smu_force_performance_level(struct smu_context *smu, enum 
> amd_dpm_forced_level level);  int smu_set_display_count(struct 
> smu_context *smu, uint32_t count);  bool smu_clk_dpm_is_enabled(struct 
> smu_context *smu, enum smu_clk_type clk_type); -int 
> smu_feature_update_enable_state(struct smu_context *smu, uint64_t 
> feature_mask, bool enabled);  const char *smu_get_message_name(struct 
> smu_context *smu, enum smu_message_type type);  const char 
> *smu_get_feature_name(struct smu_context *smu, enum smu_feature_mask 
> feature);  size_t smu_sys_get_pp_feature_mask(struct
> smu_context *smu, char *buf);
> --
> 2.23.0