Hi Slava,

Ah yes, I copied it from another function (because the struct path to the pp functions is annoying to look up hehehehe).

Thanks!

I'll submit a v2 in a bit.

Cheers,
Tom

On 06/20/2018 10:41 AM, Abramov, Slava wrote:
Should the comment then say 'get the load' instead of 'get the temperature'?

------------------------------------------------------------------------
*From:* StDenis, Tom
*Sent:* Wednesday, June 20, 2018 10:39:25 AM
*To:* Abramov, Slava; [email protected]
*Subject:* Re: [PATCH] drm/amd/amdgpu: Add a GPU_LOAD entry to sysfs


On 06/20/2018 10:37 AM, Abramov, Slava wrote:
I see some functions in amdgpu_pm.c have function level documentation, so that it would be good to have this for newly added functions.

Sure I can add some comments/docs.

Another comment is inline.


From: amd-gfx <[email protected]> on behalf of Tom St Denis 
<[email protected]>

  >Sent: Wednesday, June 20, 2018 8:31 AM
  >To: [email protected]
  >Cc: StDenis, Tom
  >Subject: [PATCH] drm/amd/amdgpu: Add a GPU_LOAD entry to sysfs
  >
  >This adds what should be a stable interface to read GPU
  >load from userspace.
  >
  >Signed-off-by: Tom St Denis <[email protected]>
  >---
  > drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 41 ++++++++++++++++++++++++++++++++++
  > 1 file changed, 41 insertions(+)
  >
  >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
  >index 113edffb5960..d57b414ac228 100644
  >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
  >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
  >@@ -918,6 +918,37 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev,
  >         return -EINVAL;
  > }
  >
  >+static ssize_t amdgpu_get_busy_level(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;
  >+
  >+       /* get the temperature */

Is load is the same thing as temperature?


Nope, there is a separate sensor for that but it is included in hwmon
and Alex would rather not duplicate it.

GPU_LOAD is a value returned by firmware based on the RLC busy status (I
think...).

Tom
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to