On Wed, Apr 11, 2018 at 2:31 AM, Rex Zhu <rex....@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.deuc...@amd.com> > Signed-off-by: Rex Zhu <rex....@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@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