[PATCH] drm/amd/amdgpu: Update read_sensor calls to have size parameter (v2)
Deucher, Alexander
Alexander.Deucher at amd.com
Fri Feb 10 17:17:27 UTC 2017
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Friday, February 10, 2017 7:18 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: StDenis, Tom
> Subject: [PATCH] drm/amd/amdgpu: Update read_sensor calls to have size
> parameter (v2)
>
> This update allows sensors to return more than 1 value and
> indicates to the caller how many bytes are written.
>
> The debugfs interface has been updated to handle reading all
> of the values. Simply seek to the enum value (multiplied
> by 4) and then read as many bytes as the sensor provides.
>
> (v2): Don't set size to 4 before reading GPU_POWER
>
> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 26
> +++++++++++++++------
> drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h | 4 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 28 +++++++++++++---
> -------
> drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 5 ++--
> drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 8 ++++++-
> drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 16
> ++++++++++++-
> drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h | 2 +-
> drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 2 +-
> 8 files changed, 64 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 0d33bc94afb5..78d1f4045539 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3192,24 +3192,36 @@ static ssize_t
> amdgpu_debugfs_sensor_read(struct file *f, char __user *buf,
> size_t size, loff_t *pos)
> {
> struct amdgpu_device *adev = f->f_inode->i_private;
> - int idx, r;
> - int32_t value;
> + int idx, x, outsize, r, valuesize;
> + uint32_t values[16];
>
> - if (size != 4 || *pos & 0x3)
> + if (size & 3 || *pos & 0x3)
> return -EINVAL;
>
> /* convert offset to sensor number */
> idx = *pos >> 2;
>
> + valuesize = sizeof(values);
> if (adev->powerplay.pp_funcs && adev->powerplay.pp_funcs-
> >read_sensor)
> - r = adev->powerplay.pp_funcs->read_sensor(adev-
> >powerplay.pp_handle, idx, &value);
> + r = adev->powerplay.pp_funcs->read_sensor(adev-
> >powerplay.pp_handle, idx, &values[0], &valuesize);
> else
> return -EINVAL;
>
> - if (!r)
> - r = put_user(value, (int32_t *)buf);
> + if (size > valuesize)
> + return -EINVAL;
> +
> + outsize = 0;
> + x = 0;
> + if (!r) {
> + while (size) {
> + r = put_user(values[x++], (int32_t *)buf);
> + buf += 4;
> + size -= 4;
> + outsize += 4;
> + }
> + }
>
> - return !r ? 4 : r;
> + return !r ? outsize : r;
> }
>
> static ssize_t amdgpu_debugfs_wave_read(struct file *f, char __user *buf,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> index 14fef5cf3566..98698dcf15c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> @@ -290,9 +290,9 @@ struct amdgpu_dpm_funcs {
> #define amdgpu_dpm_vblank_too_short(adev) (adev)->pm.funcs-
> >vblank_too_short((adev))
> #define amdgpu_dpm_enable_bapm(adev, e) (adev)->pm.funcs-
> >enable_bapm((adev), (e))
>
> -#define amdgpu_dpm_read_sensor(adev, idx, value) \
> +#define amdgpu_dpm_read_sensor(adev, idx, value, size) \
> ((adev)->pp_enabled ? \
> - (adev)->powerplay.pp_funcs->read_sensor(adev-
> >powerplay.pp_handle, (idx), (value)) : \
> + (adev)->powerplay.pp_funcs->read_sensor(adev-
> >powerplay.pp_handle, (idx), (value), (size)) : \
> -EINVAL)
>
> #define amdgpu_dpm_get_temperature(adev) \
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 392bc716e4bd..e27d2ef7531b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -1532,6 +1532,7 @@ static int amdgpu_debugfs_pm_info_pp(struct
> seq_file *m, struct amdgpu_device *a
> {
> uint32_t value;
> struct pp_gpu_power query = {0};
> + int size;
>
> /* sanity check PP is enabled */
> if (!(adev->powerplay.pp_funcs &&
> @@ -1539,16 +1540,18 @@ static int amdgpu_debugfs_pm_info_pp(struct
> seq_file *m, struct amdgpu_device *a
> return -EINVAL;
>
> /* GPU Clocks */
> + size = sizeof(value);
> seq_printf(m, "GFX Clocks and Power:\n");
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GFX_MCLK, (void *)&value))
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GFX_MCLK, (void *)&value, &size))
> seq_printf(m, "\t%u MHz (MCLK)\n", value/100);
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GFX_SCLK, (void *)&value))
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GFX_SCLK, (void *)&value, &size))
> seq_printf(m, "\t%u MHz (SCLK)\n", value/100);
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VDDGFX, (void *)&value))
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VDDGFX, (void *)&value, &size))
> seq_printf(m, "\t%u mV (VDDGFX)\n", value);
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VDDNB, (void *)&value))
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VDDNB, (void *)&value, &size))
> seq_printf(m, "\t%u mV (VDDNB)\n", value);
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_POWER, (void *)&query)) {
> + 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,
> @@ -1558,38 +1561,39 @@ static int amdgpu_debugfs_pm_info_pp(struct
> seq_file *m, struct amdgpu_device *a
> seq_printf(m, "\t%u.%u W (average GPU)\n",
> query.average_gpu_power >> 8,
> query.average_gpu_power & 0xff);
> }
> + size = sizeof(value);
> seq_printf(m, "\n");
>
> /* GPU Temp */
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_TEMP, (void *)&value))
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_TEMP, (void *)&value, &size))
> seq_printf(m, "GPU Temperature: %u C\n", value/1000);
>
> /* GPU Load */
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_LOAD, (void *)&value))
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_LOAD, (void *)&value, &size))
> seq_printf(m, "GPU Load: %u %%\n", value);
> seq_printf(m, "\n");
>
> /* UVD clocks */
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_POWER, (void *)&value)) {
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_POWER, (void *)&value, &size)) {
> if (!value) {
> seq_printf(m, "UVD: Disabled\n");
> } else {
> seq_printf(m, "UVD: Enabled\n");
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_DCLK, (void *)&value))
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_DCLK, (void *)&value, &size))
> seq_printf(m, "\t%u MHz (DCLK)\n",
> value/100);
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_VCLK, (void *)&value))
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_VCLK, (void *)&value, &size))
> seq_printf(m, "\t%u MHz (VCLK)\n",
> value/100);
> }
> }
> seq_printf(m, "\n");
>
> /* VCE clocks */
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VCE_POWER, (void *)&value)) {
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VCE_POWER, (void *)&value, &size)) {
> if (!value) {
> seq_printf(m, "VCE: Disabled\n");
> } else {
> seq_printf(m, "VCE: Enabled\n");
> - if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VCE_ECCLK, (void *)&value))
> + if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VCE_ECCLK, (void *)&value, &size))
> seq_printf(m, "\t%u MHz (ECCLK)\n",
> value/100);
> }
> }
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index 81e6856ffa11..fde8fcd46b58 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -880,7 +880,8 @@ static int pp_dpm_set_mclk_od(void *handle,
> uint32_t value)
> return hwmgr->hwmgr_func->set_mclk_od(hwmgr, value);
> }
>
> -static int pp_dpm_read_sensor(void *handle, int idx, void *value)
> +static int pp_dpm_read_sensor(void *handle, int idx,
> + void *value, int *size)
> {
> struct pp_hwmgr *hwmgr;
> struct pp_instance *pp_handle = (struct pp_instance *)handle;
> @@ -898,7 +899,7 @@ static int pp_dpm_read_sensor(void *handle, int idx,
> void *value)
> return 0;
> }
>
> - return hwmgr->hwmgr_func->read_sensor(hwmgr, idx, value);
> + return hwmgr->hwmgr_func->read_sensor(hwmgr, idx, value, size);
> }
>
> static struct amd_vce_state*
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> index edc3029df785..7aa5ca815a3a 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> @@ -1813,7 +1813,8 @@ static int cz_thermal_get_temperature(struct
> pp_hwmgr *hwmgr)
> return actual_temp;
> }
>
> -static int cz_read_sensor(struct pp_hwmgr *hwmgr, int idx, void *value)
> +static int cz_read_sensor(struct pp_hwmgr *hwmgr, int idx,
> + void *value, int *size)
> {
> struct cz_hwmgr *cz_hwmgr = (struct cz_hwmgr *)(hwmgr-
> >backend);
>
> @@ -1837,6 +1838,11 @@ static int cz_read_sensor(struct pp_hwmgr
> *hwmgr, int idx, void *value)
> uint16_t vddnb, vddgfx;
> int result;
>
> + /* size must be at least 4 bytes for all sensors */
> + if (*size < 4)
> + return -EINVAL;
> + *size = 4;
> +
> switch (idx) {
> case AMDGPU_PP_SENSOR_GFX_SCLK:
> if (sclk_index < NUM_SCLK_LEVELS) {
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index cc7506d8f3b0..ba71be12aa94 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -3311,22 +3311,29 @@ static int smu7_get_gpu_power(struct
> pp_hwmgr *hwmgr,
> return 0;
> }
>
> -static int smu7_read_sensor(struct pp_hwmgr *hwmgr, int idx, void *value)
> +static int smu7_read_sensor(struct pp_hwmgr *hwmgr, int idx,
> + void *value, int *size)
> {
> uint32_t sclk, mclk, activity_percent;
> uint32_t offset;
> struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr-
> >backend);
>
> + /* size must be at least 4 bytes for all sensors */
> + if (*size < 4)
> + return -EINVAL;
> +
> switch (idx) {
> case AMDGPU_PP_SENSOR_GFX_SCLK:
> smum_send_msg_to_smc(hwmgr->smumgr,
> PPSMC_MSG_API_GetSclkFrequency);
> sclk = cgs_read_register(hwmgr->device,
> mmSMC_MSG_ARG_0);
> *((uint32_t *)value) = sclk;
> + *size = 4;
> return 0;
> case AMDGPU_PP_SENSOR_GFX_MCLK:
> smum_send_msg_to_smc(hwmgr->smumgr,
> PPSMC_MSG_API_GetMclkFrequency);
> mclk = cgs_read_register(hwmgr->device,
> mmSMC_MSG_ARG_0);
> *((uint32_t *)value) = mclk;
> + *size = 4;
> return 0;
> case AMDGPU_PP_SENSOR_GPU_LOAD:
> offset = data->soft_regs_start +
> smum_get_offsetof(hwmgr->smumgr,
> @@ -3337,17 +3344,24 @@ static int smu7_read_sensor(struct pp_hwmgr
> *hwmgr, int idx, void *value)
> activity_percent += 0x80;
> activity_percent >>= 8;
> *((uint32_t *)value) = activity_percent > 100 ? 100 :
> activity_percent;
> + *size = 4;
> return 0;
> case AMDGPU_PP_SENSOR_GPU_TEMP:
> *((uint32_t *)value) =
> smu7_thermal_get_temperature(hwmgr);
> + *size = 4;
> return 0;
> case AMDGPU_PP_SENSOR_UVD_POWER:
> *((uint32_t *)value) = data->uvd_power_gated ? 0 : 1;
> + *size = 4;
> return 0;
> case AMDGPU_PP_SENSOR_VCE_POWER:
> *((uint32_t *)value) = data->vce_power_gated ? 0 : 1;
> + *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);
> default:
> return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> index ab99013eba77..c0bf3af6846d 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> @@ -367,7 +367,7 @@ struct amd_powerplay_funcs {
> int (*set_sclk_od)(void *handle, uint32_t value);
> int (*get_mclk_od)(void *handle);
> int (*set_mclk_od)(void *handle, uint32_t value);
> - int (*read_sensor)(void *handle, int idx, void *value);
> + int (*read_sensor)(void *handle, int idx, void *value, int *size);
> struct amd_vce_state* (*get_vce_clock_state)(void *handle,
> unsigned idx);
> int (*reset_power_profile_state)(void *handle,
> struct amd_pp_profile *request);
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> index fa3bf50eff82..8cf5aed055b6 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -355,7 +355,7 @@ struct pp_hwmgr_func {
> int (*set_sclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
> int (*get_mclk_od)(struct pp_hwmgr *hwmgr);
> int (*set_mclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
> - int (*read_sensor)(struct pp_hwmgr *hwmgr, int idx, void *value);
> + int (*read_sensor)(struct pp_hwmgr *hwmgr, int idx, void *value, int
> *size);
> int (*request_firmware)(struct pp_hwmgr *hwmgr);
> int (*release_firmware)(struct pp_hwmgr *hwmgr);
> int (*set_power_profile_state)(struct pp_hwmgr *hwmgr,
> --
> 2.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list