[PATCH] drm/amd/amdgpu: Add a GPU_LOAD entry to sysfs (v2)

[PATCH] drm/amd/amdgpu: Add a GPU_LOAD entry to sysfs (v2)

[Tom St Denis]

This adds what should be a stable interface to read GPU
load from userspace.

(v2): Fix comments and name of file per recommendations.

Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 47 ++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 113edffb5960..49138ac2be24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -918,6 +918,43 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev,
 	return -EINVAL;
 }
 
+/**
+ * DOC: busy_percent
+ *
+ * The firmware computes a percentage of load based on the activity
+ * level in the IP cores.
+ */
+static ssize_t amdgpu_get_busy_percent(struct device *dev,
+		struct device_attribute *attr,
+		char *buf)
+{
+	struct drm_device *ddev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = ddev->dev_private;
+	int r, value, size = sizeof(value);
+
+	/* sanity check PP is enabled */
+	if (!(adev->powerplay.pp_funcs &&
+	      adev->powerplay.pp_funcs->read_sensor))
+		return -EINVAL;
+
+	/* read the IP busy sensor */
+	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_LOAD,
+				   (void *)&value, &size);
+	if (r)
+		return r;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+static ssize_t amdgpu_set_busy_percent(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf,
+		size_t count)
+{
+	return -EINVAL;
+}
+
+
 static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, amdgpu_set_dpm_state);
 static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
 		   amdgpu_get_dpm_forced_performance_level,
@@ -951,6 +988,8 @@ static DEVICE_ATTR(pp_power_profile_mode, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
 		amdgpu_get_pp_od_clk_voltage,
 		amdgpu_set_pp_od_clk_voltage);
+static DEVICE_ATTR(gpu_busy_percent, S_IRUGO | S_IWUSR,
+		amdgpu_get_busy_percent, amdgpu_set_busy_percent);
 
 static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
 				      struct device_attribute *attr,
@@ -1854,6 +1893,13 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
 				"pp_od_clk_voltage\n");
 		return ret;
 	}
+	ret = device_create_file(adev->dev,
+			&dev_attr_gpu_busy_percent);
+	if (ret) {
+		DRM_ERROR("failed to create device file	"
+				"gpu_busy_level\n");
+		return ret;
+	}
 	ret = amdgpu_debugfs_pm_init(adev);
 	if (ret) {
 		DRM_ERROR("Failed to register debugfs file for dpm!\n");
@@ -1889,6 +1935,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
 			&dev_attr_pp_power_profile_mode);
 	device_remove_file(adev->dev,
 			&dev_attr_pp_od_clk_voltage);
+	device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
 }
 
 void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)
-- 
2.14.4

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

Re: [PATCH] drm/amd/amdgpu: Add a GPU_LOAD entry to sysfs (v2)

[Abramov, Slava]

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

Acked-by: Slava Abramov <slava.abramov-5C7GfCeVMHo@public.gmane.org>

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org>
Sent: Wednesday, June 20, 2018 12:48:52 PM
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Cc: StDenis, Tom
Subject: [PATCH] drm/amd/amdgpu: Add a GPU_LOAD entry to sysfs (v2)

This adds what should be a stable interface to read GPU
load from userspace.

(v2): Fix comments and name of file per recommendations.

Signed-off-by: Tom St Denis <tom.stdenis-5C7GfCeVMHo@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 47 ++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 113edffb5960..49138ac2be24 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -918,6 +918,43 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev,
         return -EINVAL;
 }

+/**
+ * DOC: busy_percent
+ *
+ * The firmware computes a percentage of load based on the activity
+ * level in the IP cores.
+ */
+static ssize_t amdgpu_get_busy_percent(struct device *dev,
+               struct device_attribute *attr,
+               char *buf)
+{
+       struct drm_device *ddev = dev_get_drvdata(dev);
+       struct amdgpu_device *adev = ddev->dev_private;
+       int r, value, size = sizeof(value);
+
+       /* sanity check PP is enabled */
+       if (!(adev->powerplay.pp_funcs &&
+             adev->powerplay.pp_funcs->read_sensor))
+               return -EINVAL;
+
+       /* read the IP busy sensor */
+       r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_LOAD,
+                                  (void *)&value, &size);
+       if (r)
+               return r;
+
+       return snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+static ssize_t amdgpu_set_busy_percent(struct device *dev,
+               struct device_attribute *attr,
+               const char *buf,
+               size_t count)
+{
+       return -EINVAL;
+}
+
+
 static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, amdgpu_set_dpm_state);
 static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
                    amdgpu_get_dpm_forced_performance_level,
@@ -951,6 +988,8 @@ static DEVICE_ATTR(pp_power_profile_mode, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
                 amdgpu_get_pp_od_clk_voltage,
                 amdgpu_set_pp_od_clk_voltage);
+static DEVICE_ATTR(gpu_busy_percent, S_IRUGO | S_IWUSR,
+               amdgpu_get_busy_percent, amdgpu_set_busy_percent);

 static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
                                       struct device_attribute *attr,
@@ -1854,6 +1893,13 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
                                 "pp_od_clk_voltage\n");
                 return ret;
         }
+       ret = device_create_file(adev->dev,
+                       &dev_attr_gpu_busy_percent);
+       if (ret) {
+               DRM_ERROR("failed to create device file "
+                               "gpu_busy_level\n");
+               return ret;
+       }
         ret = amdgpu_debugfs_pm_init(adev);
         if (ret) {
                 DRM_ERROR("Failed to register debugfs file for dpm!\n");
@@ -1889,6 +1935,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
                         &dev_attr_pp_power_profile_mode);
         device_remove_file(adev->dev,
                         &dev_attr_pp_od_clk_voltage);
+       device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
 }

 void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)
--
2.14.4

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

[-- Attachment #1.2: Type: text/html, Size: 8672 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

Re: [PATCH] drm/amd/amdgpu: Add a GPU_LOAD entry to sysfs (v2)

[Alex Deucher]

On Wed, Jun 20, 2018 at 12:48 PM, Tom St Denis <tom.stdenis@amd.com> wrote:
> This adds what should be a stable interface to read GPU
> load from userspace.
>
> (v2): Fix comments and name of file per recommendations.
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 47 ++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 113edffb5960..49138ac2be24 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -918,6 +918,43 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev,
>         return -EINVAL;
>  }
>
> +/**
> + * DOC: busy_percent
> + *
> + * The firmware computes a percentage of load based on the activity
> + * level in the IP cores.
> + */

I would reword this a bit.  the tag next to the DOC is only for
reference purposes, it doesn't show up when you reference a DOC
section in the amdgpu.rst file.  In order to make it clear what file
you are referring to, you might want to so something like:

/**
 * DOC: busy_percent
 *
 * The amdgpu driver provides a sysfs API for reading how busy the GPU
is as a percentage.
 * The file gpu_busy_percent is used for this.  The SMU firmware
computes a percentage of
 * load based on the aggregate activity level in the IP cores.
 */

Bonus points if you add the reference to this in the "GPU
Power/Thermal Controls and Monitoring" chapter in amdgpu.rst in
Documentation/gpu/.  But that can be a separate follow on patch.

With the fix above, patch is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>


> +static ssize_t amdgpu_get_busy_percent(struct device *dev,
> +               struct device_attribute *attr,
> +               char *buf)
> +{
> +       struct drm_device *ddev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = ddev->dev_private;
> +       int r, value, size = sizeof(value);
> +
> +       /* sanity check PP is enabled */
> +       if (!(adev->powerplay.pp_funcs &&
> +             adev->powerplay.pp_funcs->read_sensor))
> +               return -EINVAL;
> +
> +       /* read the IP busy sensor */
> +       r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_LOAD,
> +                                  (void *)&value, &size);
> +       if (r)
> +               return r;
> +
> +       return snprintf(buf, PAGE_SIZE, "%d\n", value);
> +}
> +
> +static ssize_t amdgpu_set_busy_percent(struct device *dev,
> +               struct device_attribute *attr,
> +               const char *buf,
> +               size_t count)
> +{
> +       return -EINVAL;
> +}
> +
> +
>  static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, amdgpu_set_dpm_state);
>  static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
>                    amdgpu_get_dpm_forced_performance_level,
> @@ -951,6 +988,8 @@ static DEVICE_ATTR(pp_power_profile_mode, S_IRUGO | S_IWUSR,
>  static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
>                 amdgpu_get_pp_od_clk_voltage,
>                 amdgpu_set_pp_od_clk_voltage);
> +static DEVICE_ATTR(gpu_busy_percent, S_IRUGO | S_IWUSR,
> +               amdgpu_get_busy_percent, amdgpu_set_busy_percent);
>
>  static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
>                                       struct device_attribute *attr,
> @@ -1854,6 +1893,13 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>                                 "pp_od_clk_voltage\n");
>                 return ret;
>         }
> +       ret = device_create_file(adev->dev,
> +                       &dev_attr_gpu_busy_percent);
> +       if (ret) {
> +               DRM_ERROR("failed to create device file "
> +                               "gpu_busy_level\n");
> +               return ret;
> +       }
>         ret = amdgpu_debugfs_pm_init(adev);
>         if (ret) {
>                 DRM_ERROR("Failed to register debugfs file for dpm!\n");
> @@ -1889,6 +1935,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>                         &dev_attr_pp_power_profile_mode);
>         device_remove_file(adev->dev,
>                         &dev_attr_pp_od_clk_voltage);
> +       device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
>  }
>
>  void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)
> --
> 2.14.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: Add a GPU_LOAD entry to sysfs (v2)

[Christian König]

Am 20.06.2018 um 18:48 schrieb Tom St Denis:
> This adds what should be a stable interface to read GPU
> load from userspace.
>
> (v2): Fix comments and name of file per recommendations.
>
> Signed-off-by: Tom St Denis <tom.stdenis@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 47 ++++++++++++++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 113edffb5960..49138ac2be24 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -918,6 +918,43 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev,
>   	return -EINVAL;
>   }
>   
> +/**
> + * DOC: busy_percent
> + *
> + * The firmware computes a percentage of load based on the activity
> + * level in the IP cores.
> + */
> +static ssize_t amdgpu_get_busy_percent(struct device *dev,
> +		struct device_attribute *attr,
> +		char *buf)
> +{
> +	struct drm_device *ddev = dev_get_drvdata(dev);
> +	struct amdgpu_device *adev = ddev->dev_private;
> +	int r, value, size = sizeof(value);
> +
> +	/* sanity check PP is enabled */
> +	if (!(adev->powerplay.pp_funcs &&
> +	      adev->powerplay.pp_funcs->read_sensor))
> +		return -EINVAL;
> +
> +	/* read the IP busy sensor */
> +	r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_GPU_LOAD,
> +				   (void *)&value, &size);
> +	if (r)
> +		return r;
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", value);
> +}
> +
> +static ssize_t amdgpu_set_busy_percent(struct device *dev,
> +		struct device_attribute *attr,
> +		const char *buf,
> +		size_t count)
> +{
> +	return -EINVAL;
> +}
> +
> +

For read only attributes the set function can be specified as NULL as well.

Apart from that this looks good to me,
Christian.

>   static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, amdgpu_set_dpm_state);
>   static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
>   		   amdgpu_get_dpm_forced_performance_level,
> @@ -951,6 +988,8 @@ static DEVICE_ATTR(pp_power_profile_mode, S_IRUGO | S_IWUSR,
>   static DEVICE_ATTR(pp_od_clk_voltage, S_IRUGO | S_IWUSR,
>   		amdgpu_get_pp_od_clk_voltage,
>   		amdgpu_set_pp_od_clk_voltage);
> +static DEVICE_ATTR(gpu_busy_percent, S_IRUGO | S_IWUSR,
> +		amdgpu_get_busy_percent, amdgpu_set_busy_percent);
>   
>   static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
>   				      struct device_attribute *attr,
> @@ -1854,6 +1893,13 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>   				"pp_od_clk_voltage\n");
>   		return ret;
>   	}
> +	ret = device_create_file(adev->dev,
> +			&dev_attr_gpu_busy_percent);
> +	if (ret) {
> +		DRM_ERROR("failed to create device file	"
> +				"gpu_busy_level\n");
> +		return ret;
> +	}
>   	ret = amdgpu_debugfs_pm_init(adev);
>   	if (ret) {
>   		DRM_ERROR("Failed to register debugfs file for dpm!\n");
> @@ -1889,6 +1935,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>   			&dev_attr_pp_power_profile_mode);
>   	device_remove_file(adev->dev,
>   			&dev_attr_pp_od_clk_voltage);
> +	device_remove_file(adev->dev, &dev_attr_gpu_busy_percent);
>   }
>   
>   void amdgpu_pm_compute_clocks(struct amdgpu_device *adev)

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