On 04/11/2018 01:23 PM, Alex Deucher wrote:
> On Wed, Apr 11, 2018 at 2:31 AM, Rex Zhu <Rex.Zhu at amd.com> wrote:
>> This struct can't be used for all asics.
>>
>> Currently smu only calculate average gpu power in real time.
>>
>> for vddc/vddci/max power,
>> we can read them per 1ms through some registers. but
>>
>> 1. the type of return values is not unified.
>> For Vi, return type is uint. For vega, return type is float.
>>
>> 2. need to assign the unit time to calculate the average power.
>>
>> so remove this struct.
>>
>> if user need to know the power on vddc/vddci.
>> we can export them with new common interface/struct.
>>
>
> If Tom and Eric are ok with this:
> Acked-by: Alex Deucher <alexander.deucher at amd.com>
It'll break the sensor reading in umr but that's fairly minor and easy
to fix. I don't really consider the --top output of umr as mission
critical (even though it can be handy). I'm ok with the change.
Once this lands on drm-next I'll upgrade umr to support it.
Cheers,
Tom
>
>> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 7 ++-----
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 22 +++++++--------------
>> drivers/gpu/drm/amd/include/kgd_pp_interface.h | 7 -------
>> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 23 +++++++++-------------
>> drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 17 +++++++---------
>> drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c | 13 ++++--------
>> 6 files changed, 29 insertions(+), 60 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index 487d39e..1e59818 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -701,9 +701,6 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>> }
>> }
>> case AMDGPU_INFO_SENSOR: {
>> - struct pp_gpu_power query = {0};
>> - int query_size = sizeof(query);
>> -
>> if (!adev->pm.dpm_enabled)
>> return -ENOENT;
>>
>> @@ -746,10 +743,10 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file
>> /* get average GPU power */
>> if (amdgpu_dpm_read_sensor(adev,
>> AMDGPU_PP_SENSOR_GPU_POWER,
>> - (void *)&query, &query_size)) {
>> + (void *)&ui32, &ui32_size)) {
>> return -EINVAL;
>> }
>> - ui32 = query.average_gpu_power >> 8;
>> + ui32 >>= 8;
>> break;
>> case AMDGPU_INFO_SENSOR_VDDNB:
>> /* get VDDNB in millivolts */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> index e5f60fc..744f105 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> @@ -1020,8 +1020,8 @@ static ssize_t amdgpu_hwmon_show_power_avg(struct device *dev,
>> {
>> struct amdgpu_device *adev = dev_get_drvdata(dev);
>> struct drm_device *ddev = adev->ddev;
>> - struct pp_gpu_power query = {0};
>> - int r, size = sizeof(query);
>> + u32 query = 0;
>> + int r, size = sizeof(u32);
>> unsigned uw;
>>
>> /* Can't get power when the card is off */
>> @@ -1041,7 +1041,7 @@ static ssize_t amdgpu_hwmon_show_power_avg(struct device *dev,
>> return r;
>>
>> /* convert to microwatts */
>> - uw = (query.average_gpu_power >> 8) * 1000000;
>> + uw = (query >> 8) * 1000000 + (query & 0xff) * 1000;
>>
>> return snprintf(buf, PAGE_SIZE, "%u\n", uw);
>> }
>> @@ -1752,7 +1752,7 @@ void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)
>> static int amdgpu_debugfs_pm_info_pp(struct seq_file *m, struct amdgpu_device *adev)
>> {
>> uint32_t value;
>> - struct pp_gpu_power query = {0};
>> + uint32_t query = 0;
>> int size;
>>
>> /* sanity check PP is enabled */
>> @@ -1775,17 +1775,9 @@ static int amdgpu_debugfs_pm_info_pp(struct seq_file *m, struct amdgpu_device *a
>> seq_printf(m, "\t%u mV (VDDGFX)\n", value);
>> if (!amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_VDDNB, (void *)&value, &size))
>> seq_printf(m, "\t%u mV (VDDNB)\n", value);
>> - size = sizeof(query);
>> - if (!amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_POWER, (void *)&query, &size)) {
>> - seq_printf(m, "\t%u.%u W (VDDC)\n", query.vddc_power >> 8,
>> - query.vddc_power & 0xff);
>> - seq_printf(m, "\t%u.%u W (VDDCI)\n", query.vddci_power >> 8,
>> - query.vddci_power & 0xff);
>> - seq_printf(m, "\t%u.%u W (max GPU)\n", query.max_gpu_power >> 8,
>> - query.max_gpu_power & 0xff);
>> - seq_printf(m, "\t%u.%u W (average GPU)\n", query.average_gpu_power >> 8,
>> - query.average_gpu_power & 0xff);
>> - }
>> + size = sizeof(uint32_t);
>> + if (!amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_POWER, (void *)&query, &size))
>> + seq_printf(m, "\t%u.%u W (average GPU)\n", query >> 8, query & 0xff);
>> size = sizeof(value);
>> seq_printf(m, "\n");
>>
>> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
>> index 5c840c0..1bec907 100644
>> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
>> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
>> @@ -149,13 +149,6 @@ struct pp_states_info {
>> uint32_t states[16];
>> };
>>
>> -struct pp_gpu_power {
>> - uint32_t vddc_power;
>> - uint32_t vddci_power;
>> - uint32_t max_gpu_power;
>> - uint32_t average_gpu_power;
>> -};
>> -
>> #define PP_GROUP_MASK 0xF0000000
>> #define PP_GROUP_SHIFT 28
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> index 20f5a6f..494c1ff 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>> @@ -3356,36 +3356,34 @@ static int smu7_get_pp_table_entry(struct pp_hwmgr *hwmgr,
>> return 0;
>> }
>>
>> -static int smu7_get_gpu_power(struct pp_hwmgr *hwmgr,
>> - struct pp_gpu_power *query)
>> +static int smu7_get_gpu_power(struct pp_hwmgr *hwmgr, u32 *query)
>> {
>> int i;
>> + u32 tmp = 0;
>>
>> 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);
>> + tmp = cgs_read_register(hwmgr->device, mmSMC_MSG_ARG_0);
>>
>> - if (query->average_gpu_power != 0)
>> + if (tmp != 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++) {
>> + for (i = 0; i < 10; i++) {
>> mdelay(1);
>> smum_send_msg_to_smc(hwmgr, PPSMC_MSG_PmStatusLogSample);
>> - query->average_gpu_power = cgs_read_ind_register(hwmgr->device,
>> + tmp = cgs_read_ind_register(hwmgr->device,
>> CGS_IND_REG__SMC,
>> ixSMU_PM_STATUS_94);
>> - if (query->average_gpu_power != 0)
>> + if (tmp != 0)
>> break;
>> }
>> + *query = tmp;
>>
>> return 0;
>> }
>> @@ -3438,10 +3436,7 @@ static int smu7_read_sensor(struct pp_hwmgr *hwmgr, int idx,
>> *size = 4;
>> return 0;
>> case AMDGPU_PP_SENSOR_GPU_POWER:
>> - if (*size < sizeof(struct pp_gpu_power))
>> - return -EINVAL;
>> - *size = sizeof(struct pp_gpu_power);
>> - return smu7_get_gpu_power(hwmgr, (struct pp_gpu_power *)value);
>> + return smu7_get_gpu_power(hwmgr, (uint32_t *)value);
>> case AMDGPU_PP_SENSOR_VDDGFX:
>> if ((data->vr_config & 0xff) == 0x2)
>> val_vid = PHM_READ_INDIRECT_FIELD(hwmgr->device,
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
>> index 127c550..0bbc564 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
>> @@ -3781,16 +3781,18 @@ static uint32_t vega10_dpm_get_mclk(struct pp_hwmgr *hwmgr, bool low)
>> }
>>
>> static int vega10_get_gpu_power(struct pp_hwmgr *hwmgr,
>> - struct pp_gpu_power *query)
>> + uint32_t *query)
>> {
>> uint32_t value;
>>
>> + if (!query)
>> + return -EINVAL;
>> +
>> smum_send_msg_to_smc(hwmgr, PPSMC_MSG_GetCurrPkgPwr);
>> value = smum_get_argument(hwmgr);
>>
>> - /* power value is an integer */
>> - memset(query, 0, sizeof *query);
>> - query->average_gpu_power = value << 8;
>> + /* SMC returning actual watts, keep consistent with legacy asics, low 8 bit as 8 fractional bits */
>> + *query = value << 8;
>>
>> return 0;
>> }
>> @@ -3840,12 +3842,7 @@ static int vega10_read_sensor(struct pp_hwmgr *hwmgr, int idx,
>> *size = 4;
>> break;
>> case AMDGPU_PP_SENSOR_GPU_POWER:
>> - if (*size < sizeof(struct pp_gpu_power))
>> - ret = -EINVAL;
>> - else {
>> - *size = sizeof(struct pp_gpu_power);
>> - ret = vega10_get_gpu_power(hwmgr, (struct pp_gpu_power *)value);
>> - }
>> + ret = vega10_get_gpu_power(hwmgr, (uint32_t *)value);
>> break;
>> case AMDGPU_PP_SENSOR_VDDGFX:
>> val_vid = (RREG32_SOC15(SMUIO, 0, mmSMUSVI0_PLANE0_CURRENTVID) &
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
>> index 3e1ed0a..782e209 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
>> @@ -1113,8 +1113,7 @@ static uint32_t vega12_dpm_get_mclk(struct pp_hwmgr *hwmgr, bool low)
>> return (mem_clk * 100);
>> }
>>
>> -static int vega12_get_gpu_power(struct pp_hwmgr *hwmgr,
>> - struct pp_gpu_power *query)
>> +static int vega12_get_gpu_power(struct pp_hwmgr *hwmgr, uint32_t *query)
>> {
>> #if 0
>> uint32_t value;
>> @@ -1126,7 +1125,7 @@ static int vega12_get_gpu_power(struct pp_hwmgr *hwmgr,
>>
>> vega12_read_arg_from_smc(hwmgr, &value);
>> /* power value is an integer */
>> - query->average_gpu_power = value << 8;
>> + *query = value << 8;
>> #endif
>> return 0;
>> }
>> @@ -1235,12 +1234,8 @@ static int vega12_read_sensor(struct pp_hwmgr *hwmgr, int idx,
>> *size = 4;
>> break;
>> case AMDGPU_PP_SENSOR_GPU_POWER:
>> - if (*size < sizeof(struct pp_gpu_power))
>> - ret = -EINVAL;
>> - else {
>> - *size = sizeof(struct pp_gpu_power);
>> - ret = vega12_get_gpu_power(hwmgr, (struct pp_gpu_power *)value);
>> - }
>> + ret = vega12_get_gpu_power(hwmgr, (uint32_t *)value);
>> +
>> break;
>> default:
>> ret = -EINVAL;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
