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