[PATCH] drm/amd/amdgpu: Update read_sensor calls to have size parameter (v2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Tom St Denis
> Sent: Friday, February 10, 2017 7:18 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: StDenis, Tom
> Subject: [PATCH] drm/amd/amdgpu: Update read_sensor calls to have size
> parameter (v2)
> 
> This update allows sensors to return more than 1 value and
> indicates to the caller how many bytes are written.
> 
> The debugfs interface has been updated to handle reading all
> of the values.  Simply seek to the enum value (multiplied
> by 4) and then read as many bytes as the sensor provides.
> 
> (v2):  Don't set size to 4 before reading GPU_POWER
> 
> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>

Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c        | 26
> +++++++++++++++------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h           |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c            | 28 +++++++++++++---
> -------
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c     |  5 ++--
>  drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c    |  8 ++++++-
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c  | 16
> ++++++++++++-
>  drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h |  2 +-
>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h         |  2 +-
>  8 files changed, 64 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 0d33bc94afb5..78d1f4045539 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3192,24 +3192,36 @@ static ssize_t
> amdgpu_debugfs_sensor_read(struct file *f, char __user *buf,
>  					size_t size, loff_t *pos)
>  {
>  	struct amdgpu_device *adev = f->f_inode->i_private;
> -	int idx, r;
> -	int32_t value;
> +	int idx, x, outsize, r, valuesize;
> +	uint32_t values[16];
> 
> -	if (size != 4 || *pos & 0x3)
> +	if (size & 3 || *pos & 0x3)
>  		return -EINVAL;
> 
>  	/* convert offset to sensor number */
>  	idx = *pos >> 2;
> 
> +	valuesize = sizeof(values);
>  	if (adev->powerplay.pp_funcs && adev->powerplay.pp_funcs-
> >read_sensor)
> -		r = adev->powerplay.pp_funcs->read_sensor(adev-
> >powerplay.pp_handle, idx, &value);
> +		r = adev->powerplay.pp_funcs->read_sensor(adev-
> >powerplay.pp_handle, idx, &values[0], &valuesize);
>  	else
>  		return -EINVAL;
> 
> -	if (!r)
> -		r = put_user(value, (int32_t *)buf);
> +	if (size > valuesize)
> +		return -EINVAL;
> +
> +	outsize = 0;
> +	x = 0;
> +	if (!r) {
> +		while (size) {
> +			r = put_user(values[x++], (int32_t *)buf);
> +			buf += 4;
> +			size -= 4;
> +			outsize += 4;
> +		}
> +	}
> 
> -	return !r ? 4 : r;
> +	return !r ? outsize : r;
>  }
> 
>  static ssize_t amdgpu_debugfs_wave_read(struct file *f, char __user *buf,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> index 14fef5cf3566..98698dcf15c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.h
> @@ -290,9 +290,9 @@ struct amdgpu_dpm_funcs {
>  #define amdgpu_dpm_vblank_too_short(adev) (adev)->pm.funcs-
> >vblank_too_short((adev))
>  #define amdgpu_dpm_enable_bapm(adev, e) (adev)->pm.funcs-
> >enable_bapm((adev), (e))
> 
> -#define amdgpu_dpm_read_sensor(adev, idx, value) \
> +#define amdgpu_dpm_read_sensor(adev, idx, value, size) \
>  	((adev)->pp_enabled ? \
> -		(adev)->powerplay.pp_funcs->read_sensor(adev-
> >powerplay.pp_handle, (idx), (value)) : \
> +		(adev)->powerplay.pp_funcs->read_sensor(adev-
> >powerplay.pp_handle, (idx), (value), (size)) : \
>  		-EINVAL)
> 
>  #define amdgpu_dpm_get_temperature(adev) \
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 392bc716e4bd..e27d2ef7531b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -1532,6 +1532,7 @@ static int amdgpu_debugfs_pm_info_pp(struct
> seq_file *m, struct amdgpu_device *a
>  {
>  	uint32_t value;
>  	struct pp_gpu_power query = {0};
> +	int size;
> 
>  	/* sanity check PP is enabled */
>  	if (!(adev->powerplay.pp_funcs &&
> @@ -1539,16 +1540,18 @@ static int amdgpu_debugfs_pm_info_pp(struct
> seq_file *m, struct amdgpu_device *a
>  	      return -EINVAL;
> 
>  	/* GPU Clocks */
> +	size = sizeof(value);
>  	seq_printf(m, "GFX Clocks and Power:\n");
> -	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GFX_MCLK, (void *)&value))
> +	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GFX_MCLK, (void *)&value, &size))
>  		seq_printf(m, "\t%u MHz (MCLK)\n", value/100);
> -	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GFX_SCLK, (void *)&value))
> +	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GFX_SCLK, (void *)&value, &size))
>  		seq_printf(m, "\t%u MHz (SCLK)\n", value/100);
> -	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VDDGFX, (void *)&value))
> +	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VDDGFX, (void *)&value, &size))
>  		seq_printf(m, "\t%u mV (VDDGFX)\n", value);
> -	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VDDNB, (void *)&value))
> +	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VDDNB, (void *)&value, &size))
>  		seq_printf(m, "\t%u mV (VDDNB)\n", value);
> -	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_POWER, (void *)&query)) {
> +	size = sizeof(query);
> +	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_POWER, (void *)&query, &size)) {
>  		seq_printf(m, "\t%u.%u W (VDDC)\n", query.vddc_power >>
> 8,
>  				query.vddc_power & 0xff);
>  		seq_printf(m, "\t%u.%u W (VDDCI)\n", query.vddci_power
> >> 8,
> @@ -1558,38 +1561,39 @@ static int amdgpu_debugfs_pm_info_pp(struct
> seq_file *m, struct amdgpu_device *a
>  		seq_printf(m, "\t%u.%u W (average GPU)\n",
> query.average_gpu_power >> 8,
>  				query.average_gpu_power & 0xff);
>  	}
> +	size = sizeof(value);
>  	seq_printf(m, "\n");
> 
>  	/* GPU Temp */
> -	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_TEMP, (void *)&value))
> +	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_TEMP, (void *)&value, &size))
>  		seq_printf(m, "GPU Temperature: %u C\n", value/1000);
> 
>  	/* GPU Load */
> -	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_LOAD, (void *)&value))
> +	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_GPU_LOAD, (void *)&value, &size))
>  		seq_printf(m, "GPU Load: %u %%\n", value);
>  	seq_printf(m, "\n");
> 
>  	/* UVD clocks */
> -	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_POWER, (void *)&value)) {
> +	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_POWER, (void *)&value, &size)) {
>  		if (!value) {
>  			seq_printf(m, "UVD: Disabled\n");
>  		} else {
>  			seq_printf(m, "UVD: Enabled\n");
> -			if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_DCLK, (void *)&value))
> +			if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_DCLK, (void *)&value, &size))
>  				seq_printf(m, "\t%u MHz (DCLK)\n",
> value/100);
> -			if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_VCLK, (void *)&value))
> +			if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_UVD_VCLK, (void *)&value, &size))
>  				seq_printf(m, "\t%u MHz (VCLK)\n",
> value/100);
>  		}
>  	}
>  	seq_printf(m, "\n");
> 
>  	/* VCE clocks */
> -	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VCE_POWER, (void *)&value)) {
> +	if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VCE_POWER, (void *)&value, &size)) {
>  		if (!value) {
>  			seq_printf(m, "VCE: Disabled\n");
>  		} else {
>  			seq_printf(m, "VCE: Enabled\n");
> -			if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VCE_ECCLK, (void *)&value))
> +			if (!amdgpu_dpm_read_sensor(adev,
> AMDGPU_PP_SENSOR_VCE_ECCLK, (void *)&value, &size))
>  				seq_printf(m, "\t%u MHz (ECCLK)\n",
> value/100);
>  		}
>  	}
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index 81e6856ffa11..fde8fcd46b58 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -880,7 +880,8 @@ static int pp_dpm_set_mclk_od(void *handle,
> uint32_t value)
>  	return hwmgr->hwmgr_func->set_mclk_od(hwmgr, value);
>  }
> 
> -static int pp_dpm_read_sensor(void *handle, int idx, void *value)
> +static int pp_dpm_read_sensor(void *handle, int idx,
> +			      void *value, int *size)
>  {
>  	struct pp_hwmgr *hwmgr;
>  	struct pp_instance *pp_handle = (struct pp_instance *)handle;
> @@ -898,7 +899,7 @@ static int pp_dpm_read_sensor(void *handle, int idx,
> void *value)
>  		return 0;
>  	}
> 
> -	return hwmgr->hwmgr_func->read_sensor(hwmgr, idx, value);
> +	return hwmgr->hwmgr_func->read_sensor(hwmgr, idx, value, size);
>  }
> 
>  static struct amd_vce_state*
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> index edc3029df785..7aa5ca815a3a 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/cz_hwmgr.c
> @@ -1813,7 +1813,8 @@ static int cz_thermal_get_temperature(struct
> pp_hwmgr *hwmgr)
>  	return actual_temp;
>  }
> 
> -static int cz_read_sensor(struct pp_hwmgr *hwmgr, int idx, void *value)
> +static int cz_read_sensor(struct pp_hwmgr *hwmgr, int idx,
> +			  void *value, int *size)
>  {
>  	struct cz_hwmgr *cz_hwmgr = (struct cz_hwmgr *)(hwmgr-
> >backend);
> 
> @@ -1837,6 +1838,11 @@ static int cz_read_sensor(struct pp_hwmgr
> *hwmgr, int idx, void *value)
>  	uint16_t vddnb, vddgfx;
>  	int result;
> 
> +	/* size must be at least 4 bytes for all sensors */
> +	if (*size < 4)
> +		return -EINVAL;
> +	*size = 4;
> +
>  	switch (idx) {
>  	case AMDGPU_PP_SENSOR_GFX_SCLK:
>  		if (sclk_index < NUM_SCLK_LEVELS) {
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index cc7506d8f3b0..ba71be12aa94 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -3311,22 +3311,29 @@ static int smu7_get_gpu_power(struct
> pp_hwmgr *hwmgr,
>  	return 0;
>  }
> 
> -static int smu7_read_sensor(struct pp_hwmgr *hwmgr, int idx, void *value)
> +static int smu7_read_sensor(struct pp_hwmgr *hwmgr, int idx,
> +			    void *value, int *size)
>  {
>  	uint32_t sclk, mclk, activity_percent;
>  	uint32_t offset;
>  	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr-
> >backend);
> 
> +	/* size must be at least 4 bytes for all sensors */
> +	if (*size < 4)
> +		return -EINVAL;
> +
>  	switch (idx) {
>  	case AMDGPU_PP_SENSOR_GFX_SCLK:
>  		smum_send_msg_to_smc(hwmgr->smumgr,
> PPSMC_MSG_API_GetSclkFrequency);
>  		sclk = cgs_read_register(hwmgr->device,
> mmSMC_MSG_ARG_0);
>  		*((uint32_t *)value) = sclk;
> +		*size = 4;
>  		return 0;
>  	case AMDGPU_PP_SENSOR_GFX_MCLK:
>  		smum_send_msg_to_smc(hwmgr->smumgr,
> PPSMC_MSG_API_GetMclkFrequency);
>  		mclk = cgs_read_register(hwmgr->device,
> mmSMC_MSG_ARG_0);
>  		*((uint32_t *)value) = mclk;
> +		*size = 4;
>  		return 0;
>  	case AMDGPU_PP_SENSOR_GPU_LOAD:
>  		offset = data->soft_regs_start +
> smum_get_offsetof(hwmgr->smumgr,
> @@ -3337,17 +3344,24 @@ static int smu7_read_sensor(struct pp_hwmgr
> *hwmgr, int idx, void *value)
>  		activity_percent += 0x80;
>  		activity_percent >>= 8;
>  		*((uint32_t *)value) = activity_percent > 100 ? 100 :
> activity_percent;
> +		*size = 4;
>  		return 0;
>  	case AMDGPU_PP_SENSOR_GPU_TEMP:
>  		*((uint32_t *)value) =
> smu7_thermal_get_temperature(hwmgr);
> +		*size = 4;
>  		return 0;
>  	case AMDGPU_PP_SENSOR_UVD_POWER:
>  		*((uint32_t *)value) = data->uvd_power_gated ? 0 : 1;
> +		*size = 4;
>  		return 0;
>  	case AMDGPU_PP_SENSOR_VCE_POWER:
>  		*((uint32_t *)value) = data->vce_power_gated ? 0 : 1;
> +		*size = 4;
>  		return 0;
>  	case AMDGPU_PP_SENSOR_GPU_POWER:
> +		if (*size < sizeof(struct pp_gpu_power))
> +			return -EINVAL;
> +		*size = sizeof(struct pp_gpu_power);
>  		return smu7_get_gpu_power(hwmgr, (struct
> pp_gpu_power *)value);
>  	default:
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> index ab99013eba77..c0bf3af6846d 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> @@ -367,7 +367,7 @@ struct amd_powerplay_funcs {
>  	int (*set_sclk_od)(void *handle, uint32_t value);
>  	int (*get_mclk_od)(void *handle);
>  	int (*set_mclk_od)(void *handle, uint32_t value);
> -	int (*read_sensor)(void *handle, int idx, void *value);
> +	int (*read_sensor)(void *handle, int idx, void *value, int *size);
>  	struct amd_vce_state* (*get_vce_clock_state)(void *handle,
> unsigned idx);
>  	int (*reset_power_profile_state)(void *handle,
>  			struct amd_pp_profile *request);
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> index fa3bf50eff82..8cf5aed055b6 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -355,7 +355,7 @@ struct pp_hwmgr_func {
>  	int (*set_sclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
>  	int (*get_mclk_od)(struct pp_hwmgr *hwmgr);
>  	int (*set_mclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
> -	int (*read_sensor)(struct pp_hwmgr *hwmgr, int idx, void *value);
> +	int (*read_sensor)(struct pp_hwmgr *hwmgr, int idx, void *value, int
> *size);
>  	int (*request_firmware)(struct pp_hwmgr *hwmgr);
>  	int (*release_firmware)(struct pp_hwmgr *hwmgr);
>  	int (*set_power_profile_state)(struct pp_hwmgr *hwmgr,
> --
> 2.11.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux