> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Friday, February 10, 2017 7:18 AM
> To: amd-gfx@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.stde...@amd.com>

Reviewed-by: Alex Deucher <alexander.deuc...@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@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

Reply via email to