<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr">
before this patch, we have 4 apis to manage smu feature bits.
<div>the patch add a new one in them, but it is not add any feature in smu.</div>
<div><br>
</div>
<div>before your patch:</div>
<div>smu_feature_is_enabled and smu_feature_set_enabled is pair of functions,</div>
<div>after your patch:</div>
<div>smu_feature_is_enabled and smu_enable_smc_feature is pair of functions<span style="font-size: 12pt;">;</span></div>
<div><span style="font-size: 12pt;"><br>
</span></div>
<div><span style="font-size: 12pt;"></span></div>
<div>so the driver don't have scenario needs to use smu_feature_set_enabeld, </div>
<div>do you agree it?</div>
<div><br>
</div>
<div>m<span style="font-size: 12pt;">ost of the time we update SMC feature, we must update software bitmap in smu_feature structure.</span></div>
<div>if not, the smu_feature_is_enabled funciton is not work well.</div>
<div><br>
</div>
<div>
<div></div>
<div><span style="font-size: 9pt;">extern int smu_feature_init_dpm(struct smu_context *smu);</span></div>
<div><span style="font-size: 9pt;">extern int smu_feature_is_enabled(struct smu_context *smu,</span></div>
<div><span style="white-space: pre; font-size: 9pt;"></span><span style="font-size: 9pt;"> enum smu_feature_mask mask);</span></div>
<div><span style="font-size: 9pt;">extern int smu_feature_set_enabled(struct smu_context *smu,</span></div>
<div><span style="white-space: pre; font-size: 9pt;"></span><span style="font-size: 9pt;"> enum smu_feature_mask mask, bool enable);</span></div>
<div><span style="font-size: 9pt;">extern int smu_feature_is_supported(struct smu_context *smu,</span></div>
<div><span style="white-space: pre; font-size: 9pt;"></span><span style="font-size: 9pt;"> enum smu_feature_mask mask);</span></div>
<div><span style="font-size: 9pt;">extern int smu_feature_set_supported(struct smu_context *smu,</span></div>
<div><span style="white-space: pre; font-size: 9pt;"></span><span style="font-size: 9pt;"> enum smu_feature_mask mask, bool enable);</span></div>
<br>
</div>
<div>Best Regards,<br>
Kevin</div>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Evan Quan <evan.quan@amd.com><br>
<b>Sent:</b> Monday, July 22, 2019 2:34:26 PM<br>
<b>To:</b> amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Cc:</b> Quan, Evan <Evan.Quan@amd.com><br>
<b>Subject:</b> [PATCH] drm/amd/powerplay: correct confusing naming for smu_feature_set_enabled</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">It does more than updating the bitmask. In fact it enables also the<br>
feature. That's confusing. As for this, a new API is added for the<br>
feature enablement job. And smu_feature_set_enabled is updated to<br>
setting the bitmask only(as smu_feature_set_supported).<br>
<br>
Change-Id: I758e4461be34c0fcbdf19448e34180a5251926c4<br>
Signed-off-by: Evan Quan <evan.quan@amd.com><br>
---<br>
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 29 +++++++++++++------<br>
.../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 2 ++<br>
drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 2 +-<br>
drivers/gpu/drm/amd/powerplay/vega20_ppt.c | 4 +--<br>
4 files changed, 25 insertions(+), 12 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c<br>
index 4e18f33a1bab..9262883d4031 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c<br>
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c<br>
@@ -511,28 +511,39 @@ int smu_feature_set_enabled(struct smu_context *smu, enum smu_feature_mask mask,<br>
{<br>
struct smu_feature *feature = &smu->smu_feature;<br>
int feature_id;<br>
- int ret = 0;<br>
<br>
feature_id = smu_feature_get_index(smu, mask);<br>
if (feature_id < 0)<br>
return -EINVAL;<br>
<br>
- WARN_ON(feature_id > feature->feature_num);<br>
-<br>
mutex_lock(&feature->mutex);<br>
- ret = smu_feature_update_enable_state(smu, feature_id, enable);<br>
- if (ret)<br>
- goto failed;<br>
<br>
if (enable)<br>
test_and_set_bit(feature_id, feature->enabled);<br>
else<br>
test_and_clear_bit(feature_id, feature->enabled);<br>
<br>
-failed:<br>
mutex_unlock(&feature->mutex);<br>
<br>
- return ret;<br>
+ return 0;<br>
+}<br>
+<br>
+int smu_enable_smc_feature(struct smu_context *smu,<br>
+ enum smu_feature_mask mask,<br>
+ bool enable)<br>
+{<br>
+ int feature_id;<br>
+ int ret = 0;<br>
+<br>
+ feature_id = smu_feature_get_index(smu, mask);<br>
+ if (feature_id < 0)<br>
+ return -EINVAL;<br>
+<br>
+ ret = smu_feature_update_enable_state(smu, feature_id, enable);<br>
+ if (ret)<br>
+ return ret;<br>
+<br>
+ return smu_feature_set_enabled(smu, mask, enable);<br>
}<br>
<br>
int smu_feature_is_supported(struct smu_context *smu, enum smu_feature_mask mask)<br>
@@ -1186,7 +1197,7 @@ static int smu_suspend(void *handle)<br>
return ret;<br>
<br>
if (adev->in_gpu_reset && baco_feature_is_enabled) {<br>
- ret = smu_feature_set_enabled(smu, SMU_FEATURE_BACO_BIT, true);<br>
+ ret = smu_enable_smc_feature(smu, SMU_FEATURE_BACO_BIT, true);<br>
if (ret) {<br>
pr_warn("set BACO feature enabled failed, return %d\n", ret);<br>
return ret;<br>
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h<br>
index b702c9ee975f..267b879796f9 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h<br>
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h<br>
@@ -946,6 +946,8 @@ extern int smu_feature_is_enabled(struct smu_context *smu,<br>
enum smu_feature_mask mask);<br>
extern int smu_feature_set_enabled(struct smu_context *smu,<br>
enum smu_feature_mask mask, bool enable);<br>
+extern int smu_enable_smc_feature(struct smu_context *smu,<br>
+ enum smu_feature_mask mask, bool enable);<br>
extern int smu_feature_is_supported(struct smu_context *smu,<br>
enum smu_feature_mask mask);<br>
extern int smu_feature_set_supported(struct smu_context *smu,<br>
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c<br>
index e3a178403546..0f59d2178d01 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c<br>
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c<br>
@@ -1419,7 +1419,7 @@ smu_v11_0_smc_fan_control(struct smu_context *smu, bool start)<br>
if (smu_feature_is_supported(smu, SMU_FEATURE_FAN_CONTROL_BIT))<br>
return 0;<br>
<br>
- ret = smu_feature_set_enabled(smu, SMU_FEATURE_FAN_CONTROL_BIT, start);<br>
+ ret = smu_enable_smc_feature(smu, SMU_FEATURE_FAN_CONTROL_BIT, start);<br>
if (ret)<br>
pr_err("[%s]%s smc FAN CONTROL feature failed!",<br>
__func__, (start ? "Start" : "Stop"));<br>
diff --git a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c<br>
index 9ead36192787..536ff884ddca 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/vega20_ppt.c<br>
+++ b/drivers/gpu/drm/amd/powerplay/vega20_ppt.c<br>
@@ -2845,7 +2845,7 @@ static int vega20_dpm_set_uvd_enable(struct smu_context *smu, bool enable)<br>
if (enable == smu_feature_is_enabled(smu, SMU_FEATURE_DPM_UVD_BIT))<br>
return 0;<br>
<br>
- return smu_feature_set_enabled(smu, SMU_FEATURE_DPM_UVD_BIT, enable);<br>
+ return smu_enable_smc_feature(smu, SMU_FEATURE_DPM_UVD_BIT, enable);<br>
}<br>
<br>
static int vega20_dpm_set_vce_enable(struct smu_context *smu, bool enable)<br>
@@ -2856,7 +2856,7 @@ static int vega20_dpm_set_vce_enable(struct smu_context *smu, bool enable)<br>
if (enable == smu_feature_is_enabled(smu, SMU_FEATURE_DPM_VCE_BIT))<br>
return 0;<br>
<br>
- return smu_feature_set_enabled(smu, SMU_FEATURE_DPM_VCE_BIT, enable);<br>
+ return smu_enable_smc_feature(smu, SMU_FEATURE_DPM_VCE_BIT, enable);<br>
}<br>
<br>
static int vega20_get_enabled_smc_features(struct smu_context *smu,<br>
-- <br>
2.22.0<br>
<br>
_______________________________________________<br>
amd-gfx mailing list<br>
amd-gfx@lists.freedesktop.org<br>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a></div>
</span></font></div>
</body>
</html>