[PATCH 2/2] drm/amd/powerplay: simplify the code of [get|set]_activity_monitor_coeff
Wang, Kevin(Yang)
Kevin1.Wang at amd.com
Tue Apr 9 03:25:07 UTC 2019
@Quan, Evan<mailto:Evan.Quan at amd.com>
I remake a new patch v2 to fix it, please help me review it.
Thanks.
Best Regards,
Kevin
________________________________
From: Quan, Evan
Sent: Tuesday, April 9, 2019 9:33:06 AM
To: Wang, Kevin(Yang); amd-gfx at lists.freedesktop.org
Cc: Huang, Ray; Feng, Kenneth; Wang, Kevin(Yang)
Subject: RE: [PATCH 2/2] drm/amd/powerplay: simplify the code of [get|set]_activity_monitor_coeff
Comment inline
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
> Wang, Kevin(Yang)
> Sent: Monday, April 08, 2019 4:43 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Huang, Ray <Ray.Huang at amd.com>; Feng, Kenneth
> <Kenneth.Feng at amd.com>; Wang, Kevin(Yang) <Kevin1.Wang at amd.com>
> Subject: [PATCH 2/2] drm/amd/powerplay: simplify the code of
> [get|set]_activity_monitor_coeff
>
> use smu_update_table_with_arg to replace old code logic
>
> Signed-off-by: Kevin Wang <kevin1.wang at amd.com>
> ---
> .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 6 --
> drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 75 ++-----------------
> 2 files changed, 7 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> index c146b5e884f8..26a7d2c7f4fa 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
> @@ -524,12 +524,6 @@ struct smu_funcs
> struct
> dm_pp_wm_sets_with_clock_ranges_soc15 *clock_ranges);
> int (*set_od8_default_settings)(struct smu_context *smu,
> bool initialize);
> - int (*get_activity_monitor_coeff)(struct smu_context *smu,
> - uint8_t *table,
> - uint16_t workload_type);
> - int (*set_activity_monitor_coeff)(struct smu_context *smu,
> - uint8_t *table,
> - uint16_t workload_type);
> int (*conv_power_profile_to_pplib_workload)(int power_profile);
> int (*get_power_profile_mode)(struct smu_context *smu, char
> *buf);
> int (*set_power_profile_mode)(struct smu_context *smu, long
> *input, uint32_t size); diff --git
> a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 0e4b4b88af24..435cb606d7eb 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1460,62 +1460,6 @@ static int
> smu_v11_0_set_od8_default_settings(struct smu_context *smu,
> return 0;
> }
>
> -static int smu_v11_0_set_activity_monitor_coeff(struct smu_context *smu,
> - uint8_t *table, uint16_t workload_type)
> -{
> - int ret = 0;
> - memcpy(smu-
> >smu_table.tables[TABLE_ACTIVITY_MONITOR_COEFF].cpu_addr,
> - table, smu-
> >smu_table.tables[TABLE_ACTIVITY_MONITOR_COEFF].size);
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_SetDriverDramAddrHigh,
> - upper_32_bits(smu-
> >smu_table.tables[TABLE_ACTIVITY_MONITOR_COEFF].mc_address));
> - if (ret) {
> - pr_err("[%s] Attempt to Set Dram Addr High Failed!",
> __func__);
> - return ret;
> - }
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_SetDriverDramAddrLow,
> - lower_32_bits(smu-
> >smu_table.tables[TABLE_ACTIVITY_MONITOR_COEFF].mc_address));
> - if (ret) {
> - pr_err("[%s] Attempt to Set Dram Addr Low Failed!",
> __func__);
> - return ret;
> - }
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_TransferTableSmu2Dram,
> - TABLE_ACTIVITY_MONITOR_COEFF
> | (workload_type << 16));
> - if (ret) {
> - pr_err("[%s] Attempt to Transfer Table From SMU Failed!",
> __func__);
> - return ret;
> - }
> -
> - return ret;
> -}
> -
> -static int smu_v11_0_get_activity_monitor_coeff(struct smu_context *smu,
> - uint8_t *table, uint16_t workload_type)
> -{
> - int ret = 0;
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_SetDriverDramAddrHigh,
> - upper_32_bits(smu-
> >smu_table.tables[TABLE_ACTIVITY_MONITOR_COEFF].mc_address));
> - if (ret) {
> - pr_err("[%s] Attempt to Set Dram Addr High Failed!",
> __func__);
> - return ret;
> - }
> -
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_SetDriverDramAddrLow,
> - lower_32_bits(smu-
> >smu_table.tables[TABLE_ACTIVITY_MONITOR_COEFF].mc_address));
> - if (ret) {
> - pr_err("[%s] Attempt to Set Dram Addr Low Failed!",
> __func__);
> - return ret;
> - }
> -
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_TransferTableSmu2Dram,
> - TABLE_ACTIVITY_MONITOR_COEFF
> | (workload_type << 16));
> - if (ret) {
> - pr_err("[%s] Attempt to Transfer Table From SMU Failed!",
> __func__);
> - return ret;
> - }
> -
> - return ret;
> -}
> -
> static int smu_v11_0_conv_power_profile_to_pplib_workload(int
> power_profile) {
> int pplib_workload = 0;
> @@ -1584,9 +1528,8 @@ static int
> smu_v11_0_get_power_profile_mode(struct smu_context *smu, char *buf)
> for (i = 0; i <= PP_SMC_POWER_PROFILE_CUSTOM; i++) {
> /* conv PP_SMC_POWER_PROFILE* to
> WORKLOAD_PPLIB_*_BIT */
> workload_type =
> smu_v11_0_conv_power_profile_to_pplib_workload(i);
> - result = smu_v11_0_get_activity_monitor_coeff(smu,
> - (uint8_t
> *)(&activity_monitor),
> - workload_type);
> + result = smu_update_table_with_arg(smu,
> TABLE_ACTIVITY_MONITOR_COEFF,
> + workload_type,
> &activity_monitor, false);
> if (result) {
> pr_err("[%s] Failed to get activity monitor!",
> __func__);
> return result;
> @@ -1658,7 +1601,7 @@ static int
> smu_v11_0_get_power_profile_mode(struct smu_context *smu, char *buf)
> static int smu_v11_0_set_power_profile_mode(struct smu_context *smu,
> long *input, uint32_t size) {
> DpmActivityMonitorCoeffInt_t activity_monitor;
> - int workload_type, ret = 0;
> + int workload_type = 0, ret = 0;
>
> smu->power_profile_mode = input[size];
>
> @@ -1668,9 +1611,8 @@ static int
> smu_v11_0_set_power_profile_mode(struct smu_context *smu, long
> *input
> }
>
> if (smu->power_profile_mode ==
> PP_SMC_POWER_PROFILE_CUSTOM) {
> - ret = smu_v11_0_get_activity_monitor_coeff(smu,
> - (uint8_t
> *)(&activity_monitor),
> -
> WORKLOAD_PPLIB_CUSTOM_BIT);
> + ret = smu_update_table_with_arg(smu,
> TABLE_ACTIVITY_MONITOR_COEFF,
> + workload_type,
[Quan, Evan] workload_type seems not initialzed as WORKLOAD_PPLIB_CUSTOM_BIT before use. Can you confirm that?
> &activity_monitor, false);
> if (ret) {
> pr_err("[%s] Failed to get activity monitor!",
> __func__);
> return ret;
> @@ -1723,9 +1665,8 @@ static int
> smu_v11_0_set_power_profile_mode(struct smu_context *smu, long
> *input
> break;
> }
>
> - ret = smu_v11_0_set_activity_monitor_coeff(smu,
> - (uint8_t
> *)(&activity_monitor),
> -
> WORKLOAD_PPLIB_CUSTOM_BIT);
> + ret = smu_update_table_with_arg(smu,
> TABLE_ACTIVITY_MONITOR_COEFF,
> +
> WORKLOAD_PPLIB_COMPUTE_BIT, &activity_monitor, true);
> if (ret) {
> pr_err("[%s] Failed to set activity monitor!",
> __func__);
> return ret;
> @@ -1994,8 +1935,6 @@ static const struct smu_funcs smu_v11_0_funcs = {
> .get_sclk = smu_v11_0_dpm_get_sclk,
> .get_mclk = smu_v11_0_dpm_get_mclk,
> .set_od8_default_settings = smu_v11_0_set_od8_default_settings,
> - .get_activity_monitor_coeff =
> smu_v11_0_get_activity_monitor_coeff,
> - .set_activity_monitor_coeff =
> smu_v11_0_set_activity_monitor_coeff,
> .conv_power_profile_to_pplib_workload =
> smu_v11_0_conv_power_profile_to_pplib_workload,
> .get_power_profile_mode = smu_v11_0_get_power_profile_mode,
> .set_power_profile_mode = smu_v11_0_set_power_profile_mode,
> --
> 2.21.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20190409/55e62c2c/attachment-0001.html>
More information about the amd-gfx
mailing list