<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
comment inline.</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Evan Quan <evan.quan@amd.com><br>
<b>Sent:</b> Thursday, August 22, 2019 6:18 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: update cached feature enablement status V2</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">Need to update in cache feature enablement status after pp_feature<br>
settings. Another fix for the commit below:<br>
drm/amd/powerplay: implment sysfs feature status function in smu<br>
<br>
V2: update smu_feature_update_enable_state() and relates<br>
<br>
<b>Change-Id: I90e29b0d839df26825d5993212f6097c7ad4bebf </b></div>
<div class="PlainText"><b>[kevin]: this information is not neccessary for public, please remove it.</b></div>
<div class="PlainText"><b>git config gerrit.createchangeid=false<br>
</b>Signed-off-by: Evan Quan <evan.quan@amd.com><br>
---<br>
drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 104 +++++++++---------<br>
.../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 -<br>
2 files changed, 52 insertions(+), 53 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 4df7fb6eaf3c..3e1cd5d9c29e 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c<br>
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c<br>
@@ -94,6 +94,55 @@ size_t smu_sys_get_pp_feature_mask(struct smu_context *smu, char *buf)<br>
return size;<br>
}<br>
<br>
+static int smu_feature_update_enable_state(struct smu_context *smu,<br>
+ uint64_t feature_mask,<br>
+ bool enabled)<br>
+{<br>
+ struct smu_feature *feature = &smu->smu_feature;<br>
+ uint32_t feature_low = 0, feature_high = 0;<br>
+ uint64_t feature_id;<br>
+ int ret = 0;<br>
+<br>
+ if (!smu->pm_enabled)<br>
+ return ret;<br>
+<br>
+ feature_low = (feature_mask >> 0 ) & 0xffffffff;<br>
+ feature_high = (feature_mask >> 32) & 0xffffffff;<br>
+<br>
+ if (enabled) {<br>
+ ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesLow,<br>
+ feature_low);<br>
+ if (ret)<br>
+ return ret;<br>
+ ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesHigh,<br>
+ feature_high);<br>
+ if (ret)<br>
+ return ret;<br>
+ } else {<br>
+ ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesLow,<br>
+ feature_low);<br>
+ if (ret)<br>
+ return ret;<br>
+ ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesHigh,<br>
+ feature_high);<br>
+ if (ret)<br>
+ return ret;<br>
+ }<br>
+<br>
+ mutex_lock(&feature->mutex);<br>
+ for (feature_id = 0; feature_id < 64; feature_id++) {<br>
+ if (feature_mask & (1ULL << feature_id)) {<br>
+ if (enabled)<br>
+ test_and_set_bit(feature_id, feature->enabled);<br>
+ else<br>
+ test_and_clear_bit(feature_id, feature->enabled);<br>
+ }<br>
+ }</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">//[kevin]: t<span>he code logic is a little redundant.</span></div>
<div class="PlainText"><span>could you use bellow macro to replace that?</span></div>
<div class="PlainText">header : <span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; background-color: rgb(255, 255, 255); display: inline !important">linux/bitmap.h</span></div>
<div class="PlainText"> * bitmap_and(dst, src1, src2, nbits) *dst = *src1 & *src2<br>
<div> * <b>bitmap_or</b>(dst, src1, src2, nbits) *dst = *src1 | *src2<br>
</div>
<div> * bitmap_xor(dst, src1, src2, nbits) *dst = *src1 ^ *src2<br>
</div>
* <b>bitmap_andnot</b>(dst, src1, src2, nbits) *dst = *src1 & ~(*src2)</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">+ mutex_unlock(&feature->mutex);<br>
+<br>
+ return ret;<br>
+}<br>
+</div>
<div class="PlainText"><br>
int smu_sys_set_pp_feature_mask(struct smu_context *smu, uint64_t new_mask)<br>
{<br>
int ret = 0;<br>
@@ -591,41 +640,7 @@ int smu_feature_init_dpm(struct smu_context *smu)<br>
<br>
return ret;<br>
}</div>
<div class="PlainText"><br>
</div>
<div class="PlainText">[kevin]:</div>
<div class="PlainText">in this patch, i know you only want to fix not cached feature cache issue,</div>
<div class="PlainText">but in v2 patch,</div>
<div class="PlainText"><span>the patch adjust the order of code functions, it seems that this is a brand new function,</span></div>
<div class="PlainText"><span>I don't think it is necessary,</span></div>
<div class="PlainText">could you just reflect the <b>modified content</b> in the patch, which can facilitate us to trace
<b>problems </b>and <b>review</b>.</div>
<div class="PlainText"><span>thanks.<br>
</span>
<div><br>
</div>
-int smu_feature_update_enable_state(struct smu_context *smu, uint64_t feature_mask, bool enabled)<br>
-{<br>
- uint32_t feature_low = 0, feature_high = 0;<br>
- int ret = 0;<br>
<br>
- if (!smu->pm_enabled)<br>
- return ret;<br>
-<br>
- feature_low = (feature_mask >> 0 ) & 0xffffffff;<br>
- feature_high = (feature_mask >> 32) & 0xffffffff;<br>
-<br>
- if (enabled) {<br>
- ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesLow,<br>
- feature_low);<br>
- if (ret)<br>
- return ret;<br>
- ret = smu_send_smc_msg_with_param(smu, SMU_MSG_EnableSmuFeaturesHigh,<br>
- feature_high);<br>
- if (ret)<br>
- return ret;<br>
-<br>
- } else {<br>
- ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesLow,<br>
- feature_low);<br>
- if (ret)<br>
- return ret;<br>
- ret = smu_send_smc_msg_with_param(smu, SMU_MSG_DisableSmuFeaturesHigh,<br>
- feature_high);<br>
- if (ret)<br>
- return ret;<br>
-<br>
- }<br>
-<br>
- return ret;<br>
-}<br>
<br>
int smu_feature_is_enabled(struct smu_context *smu, enum smu_feature_mask mask)<br>
{<br>
@@ -651,8 +666,6 @@ 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>
- uint64_t feature_mask = 0;<br>
- int ret = 0;<br>
<br>
feature_id = smu_feature_get_index(smu, mask);<br>
if (<span style="font-family: Arial, Helvetica, sans-serif;">feature_id </span>
< 0)<br>
@@ -660,22 +673,9 @@ int smu_feature_set_enabled(struct smu_context *smu, enum smu_feature_mask mask,<br>
<br>
WARN_ON(feature_id > feature->feature_num);<br>
<br>
- feature_mask = 1ULL << feature_id;<br>
-<br>
- mutex_lock(&feature->mutex);<br>
- ret = smu_feature_update_enable_state(smu, feature_mask, 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 smu_feature_update_enable_state(smu,<br>
+ 1ULL << feature_id,<br>
+ enable);<br>
}<br>
<br>
int smu_feature_is_supported(struct smu_context *smu, enum smu_feature_mask mask)<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 e80c81552d29..fbf68fd42b93 100644<br>
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h<br>
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h<br>
@@ -807,7 +807,6 @@ enum amd_dpm_forced_level smu_get_performance_level(struct smu_context *smu);<br>
int smu_force_performance_level(struct smu_context *smu, enum amd_dpm_forced_level level);<br>
int smu_set_display_count(struct smu_context *smu, uint32_t count);<br>
bool smu_clk_dpm_is_enabled(struct smu_context *smu, enum smu_clk_type clk_type);<br>
-int smu_feature_update_enable_state(struct smu_context *smu, uint64_t feature_mask, bool enabled);<br>
const char *smu_get_message_name(struct smu_context *smu, enum smu_message_type type);<br>
const char *smu_get_feature_name(struct smu_context *smu, enum smu_feature_mask feature);<br>
size_t smu_sys_get_pp_feature_mask(struct smu_context *smu, char *buf);<br>
-- <br>
2.23.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>