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>
> 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
