Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI
This patch change the power registers reading from average to maximum. If SMU team verifies it, I am OK with it. Regards, Eric On 2018-04-11 01:21 PM, Alex Deucher wrote: On Wed, Apr 11, 2018 at 2:31 AM, Rex Zhu wrote: pkgpwr is the average gpu power of 100ms. it is calculated by firmware in real time. 1. we can send smu message PPSMC_MSG_GetCurrPkgPwr to read currentpkgpwr directly. 2. On Fiji/tonga/bonaire/hawwii, without PPSMC_MSG_GetCurrPkgPwr support. Send PPSMC_MSG_PmStatusLogStart/Sample to let smu write currentpkgpwr to ixSMU_PM_STATUS_94. driver can read pkgpwr from ixSMU_PM_STATUS_94. Signed-off-by: Rex Zhu Assuming Eric is ok with removing the other power readings, Acked-by: Alex Deucher --- drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 51 -- drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 10 +++-- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c index 388184e..20f5a6f 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c @@ -3359,30 +3359,33 @@ static int smu7_get_pp_table_entry(struct pp_hwmgr *hwmgr, static int smu7_get_gpu_power(struct pp_hwmgr *hwmgr, struct pp_gpu_power *query) { - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, - PPSMC_MSG_PmStatusLogStart), - "Failed to start pm status log!", - return -1); - - msleep_interruptible(20); - - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, - PPSMC_MSG_PmStatusLogSample), - "Failed to sample pm status log!", - return -1); - - query->vddc_power = cgs_read_ind_register(hwmgr->device, - CGS_IND_REG__SMC, - ixSMU_PM_STATUS_40); - query->vddci_power = cgs_read_ind_register(hwmgr->device, - CGS_IND_REG__SMC, - ixSMU_PM_STATUS_49); - query->max_gpu_power = cgs_read_ind_register(hwmgr->device, - CGS_IND_REG__SMC, - ixSMU_PM_STATUS_94); - query->average_gpu_power = cgs_read_ind_register(hwmgr->device, - CGS_IND_REG__SMC, - ixSMU_PM_STATUS_95); + int i; + + if (!query) + return -EINVAL; + + + memset(query, 0, sizeof *query); + + smum_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_GetCurrPkgPwr, 0); + query->average_gpu_power = cgs_read_register(hwmgr->device, mmSMC_MSG_ARG_0); + + if (query->average_gpu_power != 0) + return 0; + + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogStart); + cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, + ixSMU_PM_STATUS_94, 0); + + for (i = 0; i < 20; i++) { + mdelay(1); + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogSample); + query->average_gpu_power = cgs_read_ind_register(hwmgr->device, + CGS_IND_REG__SMC, + ixSMU_PM_STATUS_94); + if (query->average_gpu_power != 0) + break; + } return 0; } diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c index fb32a3f..10a1123 100644 --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c @@ -171,8 +171,10 @@ int smu7_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t msg) ret = PHM_READ_FIELD(hwmgr->device, SMC_RESP_0, SMC_RESP); - if (ret != 1) - pr_info("\n failed to send pre message %x ret is %d \n", msg, ret); + if (ret == 0xFE) + pr_debug("last message was not supported\n"); + else if (ret != 1) + pr_info("\n last message was failed ret is %d\n", ret); cgs_write_register(hwmgr->device, mmSMC_MESSAGE_0, msg); @@ -180,7 +182,9 @@ int smu7_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t msg) ret = PHM_READ_FIELD(hwmgr->device, SMC_RESP_0, SMC_RESP); - if (ret != 1) + if (ret == 0xFE) + pr_debug("message %x was not supported\n", msg); + else if (ret != 1) pr_info("\n failed to send message %x ret is %d \n", msg, ret); return 0; -- 1.9.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx _
Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI
On Wed, Apr 11, 2018 at 2:31 AM, Rex Zhu wrote: > pkgpwr is the average gpu power of 100ms. it is calculated by > firmware in real time. > > 1. we can send smu message PPSMC_MSG_GetCurrPkgPwr to read currentpkgpwr > directly. > > 2. On Fiji/tonga/bonaire/hawwii, without PPSMC_MSG_GetCurrPkgPwr support. >Send PPSMC_MSG_PmStatusLogStart/Sample to let smu write currentpkgpwr >to ixSMU_PM_STATUS_94. driver can read pkgpwr from ixSMU_PM_STATUS_94. > > Signed-off-by: Rex Zhu Assuming Eric is ok with removing the other power readings, Acked-by: Alex Deucher > --- > drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 51 > -- > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 10 +++-- > 2 files changed, 34 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > index 388184e..20f5a6f 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > @@ -3359,30 +3359,33 @@ static int smu7_get_pp_table_entry(struct pp_hwmgr > *hwmgr, > static int smu7_get_gpu_power(struct pp_hwmgr *hwmgr, > struct pp_gpu_power *query) > { > - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, > - PPSMC_MSG_PmStatusLogStart), > - "Failed to start pm status log!", > - return -1); > - > - msleep_interruptible(20); > - > - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, > - PPSMC_MSG_PmStatusLogSample), > - "Failed to sample pm status log!", > - return -1); > - > - query->vddc_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_40); > - query->vddci_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_49); > - query->max_gpu_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_94); > - query->average_gpu_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_95); > + int i; > + > + if (!query) > + return -EINVAL; > + > + > + memset(query, 0, sizeof *query); > + > + smum_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_GetCurrPkgPwr, > 0); > + query->average_gpu_power = cgs_read_register(hwmgr->device, > mmSMC_MSG_ARG_0); > + > + if (query->average_gpu_power != 0) > + return 0; > + > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogStart); > + cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, > + ixSMU_PM_STATUS_94, > 0); > + > + for (i = 0; i < 20; i++) { > + mdelay(1); > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogSample); > + query->average_gpu_power = > cgs_read_ind_register(hwmgr->device, > + CGS_IND_REG__SMC, > + ixSMU_PM_STATUS_94); > + if (query->average_gpu_power != 0) > + break; > + } > > return 0; > } > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c > b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c > index fb32a3f..10a1123 100644 > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c > @@ -171,8 +171,10 @@ int smu7_send_msg_to_smc(struct pp_hwmgr *hwmgr, > uint16_t msg) > > ret = PHM_READ_FIELD(hwmgr->device, SMC_RESP_0, SMC_RESP); > > - if (ret != 1) > - pr_info("\n failed to send pre message %x ret is %d \n", > msg, ret); > + if (ret == 0xFE) > + pr_debug("last message was not supported\n"); > + else if (ret != 1) > + pr_info("\n last message was failed ret is %d\n", ret); > > cgs_write_register(hwmgr->device, mmSMC_MESSAGE_0, msg); > > @@ -180,7 +182,9 @@ int smu7_send_msg_to_smc(struct pp_hwmgr *hwmgr, uint16_t > msg) > > ret = PHM_READ_FIELD(hwmgr->device, SMC_RESP_0, SMC_RESP); > > - if (ret != 1) > + if (ret == 0xFE) > + pr_debug("message %x was not supported\n", msg); > + else if (ret != 1) > pr_info("\n failed to send message %x ret is %d \n", msg, > ret); > > return 0; > -- > 1.9.1 > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/m
Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI
with this patch, there should be no difference between AGT and sysfs. Best Regards Rex From: Huang, JinHuiEric Sent: Thursday, April 5, 2018 12:00 AM To: Zhu, Rex; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI So you didn't compare the result with AGT, but I did. The reality is not like your speculation. Regards, Eric On 2018-04-04 11:50 AM, Zhu, Rex wrote: If user send message PPSMC_MSG_GetCurrPkgPwr, firmware will return the pkgpwr immediately as current power value. as no PPSMC_MSG_GetCurrPkgPwr support, so send message let firmware write pkgpwr to ixSMU_PM_STATUS_94, and driver delay 10ms to read ixSMU_PM_STATUS_94. I don't think there is any problem. otherwise, there is same issue on polaris/vega. I clean ixSMU_PM_STATUS_94 before let smu write it. The delay time is enough unless we got 0 from ixSMU_PM_STATUS_94 . Best Regards Rex From: amd-gfx <mailto:amd-gfx-boun...@lists.freedesktop.org> on behalf of Eric Huang <mailto:jinhuieric.hu...@amd.com> Sent: Wednesday, April 4, 2018 11:36 PM To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> Subject: Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI Sampling period is too short. The power reading value will be not aligned with AGT's. It will confuse user that why AMD provides two different power results. Regards, Eric On 2018-04-04 04:25 AM, Rex Zhu wrote: > 1. On polaris10/11/12, Sending smu message PPSMC_MSG_GetCurrPkgPwr to > read currentpkgpwr directly. > 2. On Fiji/tonga/bonaire/hawwii, no PPSMC_MSG_GetCurrPkgPwr support. > Send PPSMC_MSG_PmStatusLogStart to let smu write currentpkgpwr > to ixSMU_PM_STATUS_94. this is asynchronous. need to delay no less > than 1ms. > 3. Clean ixSMU_PM_STATUS_94 immediately when send PPSMC_MSG_PmStatusLogStart > to avoid read old value. > 4. delay 10 ms instand of 20 ms. so the result will more similar to > the output of PPSMC_MSG_GetCurrPkgPwr. > 5. remove max power/vddc/vddci output to keep consistent with vega. > 6. for vddc/vddci power, we can calculate the average value per > [10ms, 4s] in other interface if needed. > > Signed-off-by: Rex Zhu <mailto:rex@amd.com> > --- > drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 45 > +++- > 1 file changed, 21 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > index 40f2f87..c0ce672 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > @@ -3363,30 +3363,27 @@ static int smu7_get_pp_table_entry(struct pp_hwmgr > *hwmgr, > static int smu7_get_gpu_power(struct pp_hwmgr *hwmgr, >struct pp_gpu_power *query) > { > - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, > - PPSMC_MSG_PmStatusLogStart), > - "Failed to start pm status log!", > - return -1); > - > - msleep_interruptible(20); > - > - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, > - PPSMC_MSG_PmStatusLogSample), > - "Failed to sample pm status log!", > - return -1); > - > - query->vddc_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_40); > - query->vddci_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_49); > - query->max_gpu_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_94); > - query->average_gpu_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_95); > + if (!query) > + return -EINVAL; > + > + memset(query, 0, sizeof *query); > + > + if (hwmgr->chip_id >= CHIP_POLARIS10) { > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrPkgPwr); > + query->average_gpu_power = cgs_read_register(hwmgr->device, > mmSMC_MSG_ARG_0); > + } else { > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogStart); > + cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, > + ixSMU_PM_STATUS_94, 0); > + > + msleep_interruptible(10); > + > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogSample); > + > + query-&g
Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI
So you didn't compare the result with AGT, but I did. The reality is not like your speculation. Regards, Eric On 2018-04-04 11:50 AM, Zhu, Rex wrote: If user send message PPSMC_MSG_GetCurrPkgPwr, firmware will return the pkgpwr immediately as current power value. as no PPSMC_MSG_GetCurrPkgPwr support, so send message let firmware write pkgpwr to ixSMU_PM_STATUS_94, and driver delay 10ms to read ixSMU_PM_STATUS_94. I don't think there is any problem. otherwise, there is same issue on polaris/vega. I clean ixSMU_PM_STATUS_94 before let smu write it. The delay time is enough unless we got 0 from ixSMU_PM_STATUS_94 . Best Regards Rex *From:* amd-gfx on behalf of Eric Huang *Sent:* Wednesday, April 4, 2018 11:36 PM *To:* amd-gfx@lists.freedesktop.org *Subject:* Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI Sampling period is too short. The power reading value will be not aligned with AGT's. It will confuse user that why AMD provides two different power results. Regards, Eric On 2018-04-04 04:25 AM, Rex Zhu wrote: > 1. On polaris10/11/12, Sending smu message PPSMC_MSG_GetCurrPkgPwr to > read currentpkgpwr directly. > 2. On Fiji/tonga/bonaire/hawwii, no PPSMC_MSG_GetCurrPkgPwr support. > Send PPSMC_MSG_PmStatusLogStart to let smu write currentpkgpwr > to ixSMU_PM_STATUS_94. this is asynchronous. need to delay no less > than 1ms. > 3. Clean ixSMU_PM_STATUS_94 immediately when send PPSMC_MSG_PmStatusLogStart > to avoid read old value. > 4. delay 10 ms instand of 20 ms. so the result will more similar to > the output of PPSMC_MSG_GetCurrPkgPwr. > 5. remove max power/vddc/vddci output to keep consistent with vega. > 6. for vddc/vddci power, we can calculate the average value per > [10ms, 4s] in other interface if needed. > > Signed-off-by: Rex Zhu > --- > drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 45 +++- > 1 file changed, 21 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > index 40f2f87..c0ce672 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > @@ -3363,30 +3363,27 @@ static int smu7_get_pp_table_entry(struct pp_hwmgr *hwmgr, > static int smu7_get_gpu_power(struct pp_hwmgr *hwmgr, > struct pp_gpu_power *query) > { > - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, > - PPSMC_MSG_PmStatusLogStart), > - "Failed to start pm status log!", > - return -1); > - > - msleep_interruptible(20); > - > - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, > - PPSMC_MSG_PmStatusLogSample), > - "Failed to sample pm status log!", > - return -1); > - > - query->vddc_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_40); > - query->vddci_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_49); > - query->max_gpu_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_94); > - query->average_gpu_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_95); > + if (!query) > + return -EINVAL; > + > + memset(query, 0, sizeof *query); > + > + if (hwmgr->chip_id >= CHIP_POLARIS10) { > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrPkgPwr); > + query->average_gpu_power = cgs_read_register(hwmgr->device, mmSMC_MSG_ARG_0); > + } else { > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogStart); > + cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, > + ixSMU_PM_STATUS_94, 0); > + > + msleep_interruptible(10); > + > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogSample); > + > + query->average_gpu_power = cgs_read_ind_register(hwmgr->device, > + CGS_IND_REG__SMC, > + ixSMU_PM_STATUS_94); > + } > > return 0; > } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx amd-gfx Info Page - freedesktop.org <https://lists.freedesktop.org/mailman/listinfo/amd-gfx> lists.freedesktop.org Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of Conduct. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI
If user send message PPSMC_MSG_GetCurrPkgPwr, firmware will return the pkgpwr immediately as current power value. as no PPSMC_MSG_GetCurrPkgPwr support, so send message let firmware write pkgpwr to ixSMU_PM_STATUS_94, and driver delay 10ms to read ixSMU_PM_STATUS_94. I don't think there is any problem. otherwise, there is same issue on polaris/vega. I clean ixSMU_PM_STATUS_94 before let smu write it. The delay time is enough unless we got 0 from ixSMU_PM_STATUS_94 . Best Regards Rex From: amd-gfx on behalf of Eric Huang Sent: Wednesday, April 4, 2018 11:36 PM To: amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI Sampling period is too short. The power reading value will be not aligned with AGT's. It will confuse user that why AMD provides two different power results. Regards, Eric On 2018-04-04 04:25 AM, Rex Zhu wrote: > 1. On polaris10/11/12, Sending smu message PPSMC_MSG_GetCurrPkgPwr to > read currentpkgpwr directly. > 2. On Fiji/tonga/bonaire/hawwii, no PPSMC_MSG_GetCurrPkgPwr support. > Send PPSMC_MSG_PmStatusLogStart to let smu write currentpkgpwr > to ixSMU_PM_STATUS_94. this is asynchronous. need to delay no less > than 1ms. > 3. Clean ixSMU_PM_STATUS_94 immediately when send PPSMC_MSG_PmStatusLogStart > to avoid read old value. > 4. delay 10 ms instand of 20 ms. so the result will more similar to > the output of PPSMC_MSG_GetCurrPkgPwr. > 5. remove max power/vddc/vddci output to keep consistent with vega. > 6. for vddc/vddci power, we can calculate the average value per > [10ms, 4s] in other interface if needed. > > Signed-off-by: Rex Zhu > --- > drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 45 > +++- > 1 file changed, 21 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > index 40f2f87..c0ce672 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c > @@ -3363,30 +3363,27 @@ static int smu7_get_pp_table_entry(struct pp_hwmgr > *hwmgr, > static int smu7_get_gpu_power(struct pp_hwmgr *hwmgr, >struct pp_gpu_power *query) > { > - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, > - PPSMC_MSG_PmStatusLogStart), > - "Failed to start pm status log!", > - return -1); > - > - msleep_interruptible(20); > - > - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, > - PPSMC_MSG_PmStatusLogSample), > - "Failed to sample pm status log!", > - return -1); > - > - query->vddc_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_40); > - query->vddci_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_49); > - query->max_gpu_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_94); > - query->average_gpu_power = cgs_read_ind_register(hwmgr->device, > - CGS_IND_REG__SMC, > - ixSMU_PM_STATUS_95); > + if (!query) > + return -EINVAL; > + > + memset(query, 0, sizeof *query); > + > + if (hwmgr->chip_id >= CHIP_POLARIS10) { > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrPkgPwr); > + query->average_gpu_power = cgs_read_register(hwmgr->device, > mmSMC_MSG_ARG_0); > + } else { > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogStart); > + cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, > + ixSMU_PM_STATUS_94, 0); > + > + msleep_interruptible(10); > + > + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogSample); > + > + query->average_gpu_power = cgs_read_ind_register(hwmgr->device, > + CGS_IND_REG__SMC, > + ixSMU_PM_STATUS_94); > + } > >return 0; > } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx amd-gfx Info Page - freedesktop.org<https://lists.freedesktop.org/mailman/listinfo/amd-gfx> lists.freedesktop.org Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following form. Use of all freedesktop.org lists is subject to our Code of Conduct. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] drm/amd/pp: Refine get_gpu_power for VI
Sampling period is too short. The power reading value will be not aligned with AGT's. It will confuse user that why AMD provides two different power results. Regards, Eric On 2018-04-04 04:25 AM, Rex Zhu wrote: 1. On polaris10/11/12, Sending smu message PPSMC_MSG_GetCurrPkgPwr to read currentpkgpwr directly. 2. On Fiji/tonga/bonaire/hawwii, no PPSMC_MSG_GetCurrPkgPwr support. Send PPSMC_MSG_PmStatusLogStart to let smu write currentpkgpwr to ixSMU_PM_STATUS_94. this is asynchronous. need to delay no less than 1ms. 3. Clean ixSMU_PM_STATUS_94 immediately when send PPSMC_MSG_PmStatusLogStart to avoid read old value. 4. delay 10 ms instand of 20 ms. so the result will more similar to the output of PPSMC_MSG_GetCurrPkgPwr. 5. remove max power/vddc/vddci output to keep consistent with vega. 6. for vddc/vddci power, we can calculate the average value per [10ms, 4s] in other interface if needed. Signed-off-by: Rex Zhu --- drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 45 +++- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c index 40f2f87..c0ce672 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c @@ -3363,30 +3363,27 @@ static int smu7_get_pp_table_entry(struct pp_hwmgr *hwmgr, static int smu7_get_gpu_power(struct pp_hwmgr *hwmgr, struct pp_gpu_power *query) { - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, - PPSMC_MSG_PmStatusLogStart), - "Failed to start pm status log!", - return -1); - - msleep_interruptible(20); - - PP_ASSERT_WITH_CODE(!smum_send_msg_to_smc(hwmgr, - PPSMC_MSG_PmStatusLogSample), - "Failed to sample pm status log!", - return -1); - - query->vddc_power = cgs_read_ind_register(hwmgr->device, - CGS_IND_REG__SMC, - ixSMU_PM_STATUS_40); - query->vddci_power = cgs_read_ind_register(hwmgr->device, - CGS_IND_REG__SMC, - ixSMU_PM_STATUS_49); - query->max_gpu_power = cgs_read_ind_register(hwmgr->device, - CGS_IND_REG__SMC, - ixSMU_PM_STATUS_94); - query->average_gpu_power = cgs_read_ind_register(hwmgr->device, - CGS_IND_REG__SMC, - ixSMU_PM_STATUS_95); + if (!query) + return -EINVAL; + + memset(query, 0, sizeof *query); + + if (hwmgr->chip_id >= CHIP_POLARIS10) { + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrPkgPwr); + query->average_gpu_power = cgs_read_register(hwmgr->device, mmSMC_MSG_ARG_0); + } else { + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogStart); + cgs_write_ind_register(hwmgr->device, CGS_IND_REG__SMC, + ixSMU_PM_STATUS_94, 0); + + msleep_interruptible(10); + + smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogSample); + + query->average_gpu_power = cgs_read_ind_register(hwmgr->device, + CGS_IND_REG__SMC, + ixSMU_PM_STATUS_94); + } return 0; } ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx