From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 4/7 v2] drm/i915: Add current GPU freq to sysfs Date: Thu, 06 Sep 2012 15:58:52 -0700 Message-ID: References: <1346964850-2228-1-git-send-email-ben@bwidawsk.net> <1346964850-2228-5-git-send-email-ben@bwidawsk.net> <504910F0.8030906@phoronix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from shiva.chad-versace.us (209-20-75-48.static.cloud-ips.com [209.20.75.48]) by gabe.freedesktop.org (Postfix) with ESMTP id 0802A9E83F for ; Thu, 6 Sep 2012 15:58:32 -0700 (PDT) In-Reply-To: <504910F0.8030906@phoronix.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Michael Larabel Cc: intel-gfx@lists.freedesktop.org, Jacob Pan , Arjan van de Ven , martin.peres@labri.fr List-Id: intel-gfx@lists.freedesktop.org On 2012-09-06 14:09, Michael Larabel wrote: > Have you discussed this with Radeon/Nouveau to see if they would > adopt a common sysfs interface? In the past, Eugeni Dodonov and I had > talked about this with a desire to have the open-source drivers > expose > the useful information for monitoring in a similar manner for helping > userspace (current/max/min clock speeds, voltage, etc). [My personal > interest is from monitoring this information in a more standardized > way from the Phoronix Test Suite system monitoring components rather > than writing driver-specific paths like it is now.] > > Martin (CCed) has also expressed interest from the Nouveau side in > calling for some standardization. > > -- Michael We can... I did very briefly look at what Nouveau does with the hwmon interface, and what DRM provided, and thought it'd be more work than I wanted to sign up for (which is just about nothing sysfs other than display). Thanks for bringing him into the thread though. > > On 09/06/2012 03:54 PM, Ben Widawsky wrote: >> Userspace applications such as PowerTOP are interesting in being >> able to >> read the current GPU frequency. The patch itself sets up a generic >> array >> for gen6 attributes so we can easily add other items in the future >> (and >> it also happens to be just about the cleanest way to do this). >> >> The patch is a nice addition to >> commit 1ac02185dff3afac146d745ba220dc6672d1d162 >> Author: Daniel Vetter >> Date: Thu Aug 30 13:26:48 2012 +0200 >> >> drm/i915: add a tracepoint for gpu frequency changes >> >> Reading the GPU frequncy can be done by reading a file like: >> /sys/class/drm/card0/render_frequency_mhz >> >> v2: rename the sysfs file to gt_cur_freq_mhz (Daniel) >> Use kdev naming (Chris) >> Use DEVICE_ATTR over __ATTR (Ben) >> Add min/max freq (Ben/Daniel) >> user the new #define for frequency multiplier >> >> CC: Arjan van de Ven >> CC: Jacob Pan >> Signed-off-by: Ben Widawsky >> --- >> drivers/gpu/drm/i915/i915_sysfs.c | 71 >> +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 71 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c >> b/drivers/gpu/drm/i915/i915_sysfs.c >> index c701a17..6c7f905 100644 >> --- a/drivers/gpu/drm/i915/i915_sysfs.c >> +++ b/drivers/gpu/drm/i915/i915_sysfs.c >> @@ -203,6 +203,70 @@ static struct bin_attribute dpf_attrs = { >> .mmap = NULL >> }; >> +static ssize_t gt_cur_freq_mhz_show(struct device *kdev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct drm_minor *minor = container_of(kdev, struct drm_minor, >> kdev); >> + struct drm_device *dev = minor->dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + int ret; >> + >> + ret = i915_mutex_lock_interruptible(dev); >> + if (ret) >> + return ret; >> + >> + ret = dev_priv->rps.cur_delay * GT_FREQUENCY_MULTIPLIER; >> + mutex_unlock(&dev->struct_mutex); >> + >> + return snprintf(buf, PAGE_SIZE, "%d", ret); >> +} >> + >> +static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct >> device_attribute *attr, char *buf) >> +{ >> + struct drm_minor *minor = container_of(kdev, struct drm_minor, >> kdev); >> + struct drm_device *dev = minor->dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + int ret; >> + >> + ret = i915_mutex_lock_interruptible(dev); >> + if (ret) >> + return ret; >> + >> + ret = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER; >> + mutex_unlock(&dev->struct_mutex); >> + >> + return snprintf(buf, PAGE_SIZE, "%d", ret); >> +} >> + >> +static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct >> device_attribute *attr, char *buf) >> +{ >> + struct drm_minor *minor = container_of(kdev, struct drm_minor, >> kdev); >> + struct drm_device *dev = minor->dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + int ret; >> + >> + ret = i915_mutex_lock_interruptible(dev); >> + if (ret) >> + return ret; >> + >> + ret = dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER; >> + mutex_unlock(&dev->struct_mutex); >> + >> + return snprintf(buf, PAGE_SIZE, "%d", ret); >> +} >> + >> +static DEVICE_ATTR(gt_cur_freq_mhz, S_IRUGO, gt_cur_freq_mhz_show, >> NULL); >> +static DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, >> gt_max_freq_mhz_show, NULL); >> +static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, >> gt_min_freq_mhz_show, NULL); >> + >> +static const struct attribute *gen6_attrs[] = { >> + &dev_attr_gt_cur_freq_mhz.attr, >> + &dev_attr_gt_max_freq_mhz.attr, >> + &dev_attr_gt_min_freq_mhz.attr, >> + NULL, >> +}; >> + >> + >> void i915_setup_sysfs(struct drm_device *dev) >> { >> int ret; >> @@ -220,10 +284,17 @@ void i915_setup_sysfs(struct drm_device *dev) >> if (ret) >> DRM_ERROR("l3 parity sysfs setup failed\n"); >> } >> + >> + if (INTEL_INFO(dev)->gen >= 6) { >> + ret = sysfs_create_files(&dev->primary->kdev.kobj, gen6_attrs); >> + if (ret) >> + DRM_ERROR("gen6 sysfs setup failed\n"); >> + } >> } >> void i915_teardown_sysfs(struct drm_device *dev) >> { >> + sysfs_remove_files(&dev->primary->kdev.kobj, gen6_attrs); >> device_remove_bin_file(&dev->primary->kdev, &dpf_attrs); >> sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group); >> } -- Ben Widawsky, Intel Open Source Technology Center