All of lore.kernel.org
 help / color / mirror / Atom feed
* Add sensors support to debugfs for Powerplay
@ 2016-09-15 13:22 Tom St Denis
       [not found] ` <20160915132228.27765-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Tom St Denis @ 2016-09-15 13:22 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

This patch adds the ability to read various sensors from userspace.
It's extensible so we can always add more later on.

Currently CZ/ST supported (CZ tested with a WIP copy of the debug
tool).

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] drm/amd/amdgpu:  Add sensors debugfs support
       [not found] ` <20160915132228.27765-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2016-09-15 13:22   ` Tom St Denis
       [not found]     ` <20160915132228.27765-2-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Tom St Denis @ 2016-09-15 13:22 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Tom St Denis

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)
+		return -EINVAL;
+
+	/* convert offset to sensor number */
+	*pos >>= 2;
+
+	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 {
-- 
2.10.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support
       [not found]     ` <20160915132228.27765-2-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
@ 2016-09-15 13:29       ` Christian König
       [not found]         ` <ad714ffe-1366-7af5-f01b-f3cc87f20841-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
  2016-09-16  5:24       ` Edward O'Callaghan
  1 sibling, 1 reply; 8+ messages in thread
From: Christian König @ 2016-09-15 13:29 UTC (permalink / raw)
  To: Tom St Denis, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Tom St Denis

Am 15.09.2016 um 15:22 schrieb Tom St Denis:
> 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>

Might be a good idea to split this into implementing the 
backend/frontend functions, but this way works for me as well.

Acked-by: Christian König <christian.koenig@amd.com>.

Regards,
Christian.

> ---
>   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)
> +		return -EINVAL;
> +
> +	/* convert offset to sensor number */
> +	*pos >>= 2;
> +
> +	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 {


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support
       [not found]         ` <ad714ffe-1366-7af5-f01b-f3cc87f20841-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
@ 2016-09-15 13:32           ` StDenis, Tom
       [not found]             ` <CY4PR12MB176865A001D9C9891E4C09F7F7F00-rpdhrqHFk06yjjPBNVDk/QdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: StDenis, Tom @ 2016-09-15 13:32 UTC (permalink / raw)
  To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 15123 bytes --]

I don't mind splitting this into pp/headers then amdgpu_device.c.


Took a bit to get the userland formatting pretty but it works.  Can --top the PM values.  If we could get temp/watt/etc readings in the kernel I could throw those in userland too.


Tom


________________________________
From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
Sent: Thursday, September 15, 2016 09:29
To: Tom St Denis; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: StDenis, Tom
Subject: Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support

Am 15.09.2016 um 15:22 schrieb Tom St Denis:
> 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-5C7GfCeVMHo@public.gmane.org>

Might be a good idea to split this into implementing the
backend/frontend functions, but this way works for me as well.

Acked-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>.

Regards,
Christian.

> ---
>   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)
> +             return -EINVAL;
> +
> +     /* convert offset to sensor number */
> +     *pos >>= 2;
> +
> +     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 {



[-- Attachment #1.2: Type: text/html, Size: 29538 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] drm/amd/amdgpu: Add sensors debugfs support
       [not found]             ` <CY4PR12MB176865A001D9C9891E4C09F7F7F00-rpdhrqHFk06yjjPBNVDk/QdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-09-15 15:10               ` Deucher, Alexander
       [not found]                 ` <MWHPR12MB1694800A23CDF4BA960751F2F7F00-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Deucher, Alexander @ 2016-09-15 15:10 UTC (permalink / raw)
  To: StDenis, Tom, Christian König,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 15787 bytes --]

FWIW, temperature and fan are already exposed via standard hwmon interfaces.  It might be better to use standard interfaces for some of these things if they exist.

Alex

From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf Of StDenis, Tom
Sent: Thursday, September 15, 2016 9:32 AM
To: Christian König; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support


I don't mind splitting this into pp/headers then amdgpu_device.c.



Took a bit to get the userland formatting pretty but it works.  Can --top the PM values.  If we could get temp/watt/etc readings in the kernel I could throw those in userland too.



Tom

________________________________
From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org<mailto:deathsimple@vodafone.de>>
Sent: Thursday, September 15, 2016 09:29
To: Tom St Denis; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lMiVNPc3mojA@public.gmane.orgsktop.org>
Cc: StDenis, Tom
Subject: Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support

Am 15.09.2016 um 15:22 schrieb Tom St Denis:
> 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-5C7GfCeVMHo@public.gmane.org<mailto:tom.stdenis@amd.com>>

Might be a good idea to split this into implementing the
backend/frontend functions, but this way works for me as well.

Acked-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org<mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>>.

Regards,
Christian.

> ---
>   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)
> +             return -EINVAL;
> +
> +     /* convert offset to sensor number */
> +     *pos >>= 2;
> +
> +     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 {


[-- Attachment #1.2: Type: text/html, Size: 34516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support
       [not found]                 ` <MWHPR12MB1694800A23CDF4BA960751F2F7F00-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-09-15 16:11                   ` StDenis, Tom
  0 siblings, 0 replies; 8+ messages in thread
From: StDenis, Tom @ 2016-09-15 16:11 UTC (permalink / raw)
  To: Deucher, Alexander, Christian König,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 16291 bytes --]

Hi Alex,


I don't know much about the hwmon API but maybe the backend could be useful for some hwmon glue?


Though I'd like to keep the frontend (debugfs) for now until we sort out hwmon changes.


Tom


________________________________
From: Deucher, Alexander
Sent: Thursday, September 15, 2016 11:10
To: StDenis, Tom; Christian König; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: RE: [PATCH] drm/amd/amdgpu: Add sensors debugfs support


FWIW, temperature and fan are already exposed via standard hwmon interfaces.  It might be better to use standard interfaces for some of these things if they exist.



Alex



From: amd-gfx [mailto:amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org] On Behalf Of StDenis, Tom
Sent: Thursday, September 15, 2016 9:32 AM
To: Christian König; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support



I don't mind splitting this into pp/headers then amdgpu_device.c.



Took a bit to get the userland formatting pretty but it works.  Can --top the PM values.  If we could get temp/watt/etc readings in the kernel I could throw those in userland too.



Tom



________________________________

From: Christian König <deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org<mailto:deathsimple@vodafone.de>>
Sent: Thursday, September 15, 2016 09:29
To: Tom St Denis; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org<mailto:amd-gfx-PD4FTy7X32lMiVNPc3mojA@public.gmane.orgsktop.org>
Cc: StDenis, Tom
Subject: Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support



Am 15.09.2016 um 15:22 schrieb Tom St Denis:
> 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-5C7GfCeVMHo@public.gmane.org<mailto:tom.stdenis@amd.com>>

Might be a good idea to split this into implementing the
backend/frontend functions, but this way works for me as well.

Acked-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org<mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>>.

Regards,
Christian.

> ---
>   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)
> +             return -EINVAL;
> +
> +     /* convert offset to sensor number */
> +     *pos >>= 2;
> +
> +     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 {


[-- Attachment #1.2: Type: text/html, Size: 34612 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support
       [not found]     ` <20160915132228.27765-2-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
  2016-09-15 13:29       ` Christian König
@ 2016-09-16  5:24       ` Edward O'Callaghan
       [not found]         ` <345b5123-c13c-eba3-e2d6-15232cd50733-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org>
  1 sibling, 1 reply; 8+ messages in thread
From: Edward O'Callaghan @ 2016-09-16  5:24 UTC (permalink / raw)
  To: Tom St Denis, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Tom St Denis


[-- Attachment #1.1.1: Type: text/plain, Size: 12906 bytes --]



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-5C7GfCeVMHo@public.gmane.org>
> ---
>  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 {
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support
       [not found]         ` <345b5123-c13c-eba3-e2d6-15232cd50733-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org>
@ 2016-09-16 11:45           ` StDenis, Tom
  0 siblings, 0 replies; 8+ messages in thread
From: StDenis, Tom @ 2016-09-16 11:45 UTC (permalink / raw)
  To: Edward O'Callaghan, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 15159 bytes --]

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


________________________________
From: Edward O'Callaghan <funfunctor-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org>
Sent: Friday, September 16, 2016 01:24
To: Tom St Denis; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: StDenis, Tom
Subject: Re: [PATCH] drm/amd/amdgpu: Add sensors debugfs support



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-5C7GfCeVMHo@public.gmane.org>
> ---
>  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 {
>


[-- Attachment #1.2: Type: text/html, Size: 29275 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-09-16 11:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 13:22 Add sensors support to debugfs for Powerplay Tom St Denis
     [not found] ` <20160915132228.27765-1-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-09-15 13:22   ` [PATCH] drm/amd/amdgpu: Add sensors debugfs support Tom St Denis
     [not found]     ` <20160915132228.27765-2-tom.stdenis-5C7GfCeVMHo@public.gmane.org>
2016-09-15 13:29       ` Christian König
     [not found]         ` <ad714ffe-1366-7af5-f01b-f3cc87f20841-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2016-09-15 13:32           ` StDenis, Tom
     [not found]             ` <CY4PR12MB176865A001D9C9891E4C09F7F7F00-rpdhrqHFk06yjjPBNVDk/QdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-09-15 15:10               ` Deucher, Alexander
     [not found]                 ` <MWHPR12MB1694800A23CDF4BA960751F2F7F00-Gy0DoCVfaSW4WA4dJ5YXGAdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-09-15 16:11                   ` StDenis, Tom
2016-09-16  5:24       ` Edward O'Callaghan
     [not found]         ` <345b5123-c13c-eba3-e2d6-15232cd50733-dczkZgxz+BNUPWh3PAxdjQ@public.gmane.org>
2016-09-16 11:45           ` StDenis, Tom

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.