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 <daniel.vetter at ffwll.ch>
>> 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 <arjan at linux.intel.com>
>> CC: Jacob Pan <jacob.jun.pan at linux.intel.com>
>> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>> ---
>> 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
