[PATCH 3/3] drm/amd/pp: Remove struct pp_gpu_power

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux