<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;background-color:#FFFFFF;font-family:Calibri,Arial,Helvetica,sans-serif;"> The idea was to treat it like a somewhat normal file. So every 4 bytes is a new sensor. So I do "*pos >>= 2" to map an offset to an index. Linux doesn't care that I modified *pos though because we're not allowing the user to read more than a 4 bytes at a time (so if they tried to read more than a page it would just fail anyways). Tom <div style="color: rgb(0, 0, 0);"> <div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">From: Edward O'Callaghan <funfunctor@folklore1984.net> Sent: Friday, September 16, 2016 01:24 To: Tom St Denis; amd-gfx@lists.freedesktop.org Cc: StDenis, Tom Subject: Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support <div> </div> </div> </div> <div class="PlainText"> On 09/15/2016 11:22 PM, Tom St Denis wrote: > This patch adds a callback to powerplay which > reads specific PP sensors (vdd/clocks/load) which is then > accessible via debugfs. The idea being is it'll be a standard > interface between different ASICs that userland tools can > read. > > Currently only CZ/ST is supported but the others are > NULL'ed off so they shouldn't cause any sort of oops. > > Signed-off-by: Tom St Denis <tom.stdenis@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 31 +++++++ > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 20 +++++ > drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c | 96 ++++++++++++++++++++++ > drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 1 + > .../gpu/drm/amd/powerplay/hwmgr/iceland_hwmgr.c | 1 + > .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c | 1 + > drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 1 + > drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h | 10 +++ > drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 1 + > 9 files changed, 162 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 9103e7baf26e..b6a4588c95ee 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2841,6 +2841,29 @@ static ssize_t amdgpu_debugfs_gca_config_read(struct file *f, char __user *buf, > return result; > } > > +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 r; > + int32_t value; > + > + if (size != 4 || *pos & 0x3) Just some minor questions, maybe I miss-read but maybe dereference pos the once and have it const? > + return -EINVAL; > + > + /* convert offset to sensor number */ > + *pos >>= 2; Is the intent here just a local mutation or a in-place global state transition? Kind Regards, Edward. > + > + if (adev->powerplay.pp_funcs && adev->powerplay.pp_funcs->read_sensor) > + r = adev->powerplay.pp_funcs->read_sensor(adev->powerplay.pp_handle, *pos, &value); > + else > + r = -EINVAL; > + > + if (!r) > + r = put_user(value, (int32_t *)buf); > + > + return !r ? 4 : r; > +} > > static const struct file_operations amdgpu_debugfs_regs_fops = { > .owner = THIS_MODULE, > @@ -2873,12 +2896,19 @@ static const struct file_operations amdgpu_debugfs_gca_config_fops = { > .llseek = default_llseek > }; > > +static const struct file_operations amdgpu_debugfs_sensors_fops = { > + .owner = THIS_MODULE, > + .read = amdgpu_debugfs_sensor_read, > + .llseek = default_llseek > +}; > + > static const struct file_operations *debugfs_regs[] = { > &amdgpu_debugfs_regs_fops, > &amdgpu_debugfs_regs_didt_fops, > &amdgpu_debugfs_regs_pcie_fops, > &amdgpu_debugfs_regs_smc_fops, > &amdgpu_debugfs_gca_config_fops, > + &amdgpu_debugfs_sensors_fops, > }; > > static const char *debugfs_regs_names[] = { > @@ -2887,6 +2917,7 @@ static const char *debugfs_regs_names[] = { > "amdgpu_regs_pcie", > "amdgpu_regs_smc", > "amdgpu_gca_config", > + "amdgpu_sensors", > }; > > static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > index b1d19409bf86..ee0368381e82 100644 > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > @@ -894,6 +894,25 @@ 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, int32_t *value) > +{ > + struct pp_hwmgr *hwmgr; > + > + if (!handle) > + return -EINVAL; > + > + hwmgr = ((struct pp_instance *)handle)->hwmgr; > + > + PP_CHECK_HW(hwmgr); > + > + if (hwmgr->hwmgr_func->read_sensor == NULL) { > + printk(KERN_INFO "%s was not implemented.\n", __func__); > + return 0; > + } > + > + return hwmgr->hwmgr_func->read_sensor(hwmgr, idx, value); > +} > + > const struct amd_powerplay_funcs pp_dpm_funcs = { > .get_temperature = pp_dpm_get_temperature, > .load_firmware = pp_dpm_load_fw, > @@ -920,6 +939,7 @@ const struct amd_powerplay_funcs pp_dpm_funcs = { > .set_sclk_od = pp_dpm_set_sclk_od, > .get_mclk_od = pp_dpm_get_mclk_od, > .set_mclk_od = pp_dpm_set_mclk_od, > + .read_sensor = pp_dpm_read_sensor, > }; > > static int amd_pp_instance_init(struct amd_pp_init *pp_init, > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c > index 5ecef1732e20..9f3c5a8a903c 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c > @@ -1857,6 +1857,101 @@ static int cz_get_max_high_clocks(struct pp_hwmgr *hwmgr, struct amd_pp_simple_c > return 0; > } > > +static int cz_read_sensor(struct pp_hwmgr *hwmgr, int idx, int32_t *value) > +{ > + struct cz_hwmgr *cz_hwmgr = (struct cz_hwmgr *)(hwmgr->backend); > + > + struct phm_clock_voltage_dependency_table *table = > + hwmgr->dyn_state.vddc_dependency_on_sclk; > + > + struct phm_vce_clock_voltage_dependency_table *vce_table = > + hwmgr->dyn_state.vce_clock_voltage_dependency_table; > + > + struct phm_uvd_clock_voltage_dependency_table *uvd_table = > + hwmgr->dyn_state.uvd_clock_voltage_dependency_table; > + > + uint32_t sclk_index = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixTARGET_AND_CURRENT_PROFILE_INDEX), > + TARGET_AND_CURRENT_PROFILE_INDEX, CURR_SCLK_INDEX); > + uint32_t uvd_index = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixTARGET_AND_CURRENT_PROFILE_INDEX_2), > + TARGET_AND_CURRENT_PROFILE_INDEX_2, CURR_UVD_INDEX); > + uint32_t vce_index = PHM_GET_FIELD(cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixTARGET_AND_CURRENT_PROFILE_INDEX_2), > + TARGET_AND_CURRENT_PROFILE_INDEX_2, CURR_VCE_INDEX); > + > + uint32_t sclk, vclk, dclk, ecclk, tmp, activity_percent; > + uint16_t vddnb, vddgfx; > + int result; > + > + switch (idx) { > + case AMDGPU_PP_SENSOR_GFX_SCLK: > + if (sclk_index < NUM_SCLK_LEVELS) { > + sclk = table->entries[sclk_index].clk; > + *value = sclk; > + return 0; > + } > + return -EINVAL; > + case AMDGPU_PP_SENSOR_VDDNB: > + tmp = (cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixSMUSVI_NB_CURRENTVID) & > + CURRENT_NB_VID_MASK) >> CURRENT_NB_VID__SHIFT; > + vddnb = cz_convert_8Bit_index_to_voltage(hwmgr, tmp); > + *value = vddnb; > + return 0; > + case AMDGPU_PP_SENSOR_VDDGFX: > + tmp = (cgs_read_ind_register(hwmgr->device, CGS_IND_REG__SMC, ixSMUSVI_GFX_CURRENTVID) & > + CURRENT_GFX_VID_MASK) >> CURRENT_GFX_VID__SHIFT; > + vddgfx = cz_convert_8Bit_index_to_voltage(hwmgr, (u16)tmp); > + *value = vddgfx; > + return 0; > + case AMDGPU_PP_SENSOR_UVD_VCLK: > + if (!cz_hwmgr->uvd_power_gated) { > + if (uvd_index >= CZ_MAX_HARDWARE_POWERLEVELS) { > + return -EINVAL; > + } else { > + vclk = uvd_table->entries[uvd_index].vclk; > + *value = vclk; > + return 0; > + } > + } > + *value = 0; > + return 0; > + case AMDGPU_PP_SENSOR_UVD_DCLK: > + if (!cz_hwmgr->uvd_power_gated) { > + if (uvd_index >= CZ_MAX_HARDWARE_POWERLEVELS) { > + return -EINVAL; > + } else { > + dclk = uvd_table->entries[uvd_index].dclk; > + *value = dclk; > + return 0; > + } > + } > + *value = 0; > + return 0; > + case AMDGPU_PP_SENSOR_VCE_ECCLK: > + if (!cz_hwmgr->vce_power_gated) { > + if (vce_index >= CZ_MAX_HARDWARE_POWERLEVELS) { > + return -EINVAL; > + } else { > + ecclk = vce_table->entries[vce_index].ecclk; > + *value = ecclk; > + return 0; > + } > + } > + *value = 0; > + return 0; > + case AMDGPU_PP_SENSOR_GPU_LOAD: > + result = smum_send_msg_to_smc(hwmgr->smumgr, PPSMC_MSG_GetAverageGraphicsActivity); > + if (0 == result) { > + activity_percent = cgs_read_register(hwmgr->device, mmSMU_MP1_SRBM2P_ARG_0); > + activity_percent = activity_percent > 100 ? 100 : activity_percent; > + } else { > + activity_percent = 50; > + } > + *value = activity_percent; > + return 0; > + default: > + return -EINVAL; > + } > +} > + > static const struct pp_hwmgr_func cz_hwmgr_funcs = { > .backend_init = cz_hwmgr_backend_init, > .backend_fini = cz_hwmgr_backend_fini, > @@ -1882,6 +1977,7 @@ static const struct pp_hwmgr_func cz_hwmgr_funcs = { > .get_current_shallow_sleep_clocks = cz_get_current_shallow_sleep_clocks, > .get_clock_by_type = cz_get_clock_by_type, > .get_max_high_clocks = cz_get_max_high_clocks, > + .read_sensor = cz_read_sensor, > }; > > int cz_hwmgr_init(struct pp_hwmgr *hwmgr) > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c > index 0d4c99b9e3f9..c64def1884c9 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c > @@ -5590,6 +5590,7 @@ static const struct pp_hwmgr_func fiji_hwmgr_funcs = { > .set_sclk_od = fiji_set_sclk_od, > .get_mclk_od = fiji_get_mclk_od, > .set_mclk_od = fiji_set_mclk_od, > + .read_sensor = NULL, > }; > > int fiji_hwmgr_init(struct pp_hwmgr *hwmgr) > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/iceland_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/iceland_hwmgr.c > index 5abe43360ec0..d7a1410402d4 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/iceland_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/iceland_hwmgr.c > @@ -5663,6 +5663,7 @@ static const struct pp_hwmgr_func iceland_hwmgr_funcs = { > .set_sclk_od = iceland_set_sclk_od, > .get_mclk_od = iceland_get_mclk_od, > .set_mclk_od = iceland_set_mclk_od, > + .read_sensor = NULL, > }; > > int iceland_hwmgr_init(struct pp_hwmgr *hwmgr) > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c > index 970e3930452d..8da1d0f66240 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c > @@ -5278,6 +5278,7 @@ static const struct pp_hwmgr_func polaris10_hwmgr_funcs = { > .set_sclk_od = polaris10_set_sclk_od, > .get_mclk_od = polaris10_get_mclk_od, > .set_mclk_od = polaris10_set_mclk_od, > + .read_sensor = NULL, > }; > > int polaris10_hwmgr_init(struct pp_hwmgr *hwmgr) > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > index 42783bf7647c..a5175ea5bb46 100644 > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c > @@ -6359,6 +6359,7 @@ static const struct pp_hwmgr_func tonga_hwmgr_funcs = { > .set_sclk_od = tonga_set_sclk_od, > .get_mclk_od = tonga_get_mclk_od, > .set_mclk_od = tonga_set_mclk_od, > + .read_sensor = NULL, > }; > > int tonga_hwmgr_init(struct pp_hwmgr *hwmgr) > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h > index 18f39e89a7aa..bafb593b568a 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h > @@ -29,6 +29,15 @@ > #include "amd_shared.h" > #include "cgs_common.h" > > +enum amd_pp_sensors { > + AMDGPU_PP_SENSOR_GFX_SCLK = 0, > + AMDGPU_PP_SENSOR_VDDNB, > + AMDGPU_PP_SENSOR_VDDGFX, > + AMDGPU_PP_SENSOR_UVD_VCLK, > + AMDGPU_PP_SENSOR_UVD_DCLK, > + AMDGPU_PP_SENSOR_VCE_ECCLK, > + AMDGPU_PP_SENSOR_GPU_LOAD, > +}; > > enum amd_pp_event { > AMD_PP_EVENT_INITIALIZE = 0, > @@ -346,6 +355,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, int32_t *value); > }; > > struct amd_powerplay { > diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > index e98748344801..310ba0ce2934 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h > @@ -359,6 +359,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, int32_t *value); > }; > > struct pp_table_func { > </div> </div> </div> </body> </html>