<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> Should the comment then say 'get the load' instead of 'get the temperature'? </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: StDenis, Tom Sent: Wednesday, June 20, 2018 10:39:25 AM To: Abramov, Slava; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/amd/amdgpu: Add a GPU_LOAD entry to sysfs <div> </div> </div> <div class="BodyFragment"> <div class="PlainText"> 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 <amd-gfx-bounces@lists.freedesktop.org> on behalf of Tom St Denis <tom.stdenis@amd.com> > > >Sent: Wednesday, June 20, 2018 8:31 AM > >To: amd-gfx@lists.freedesktop.org > >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 <tom.stdenis@amd.com> > >--- > > 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 </div> </div> </body> </html>