[Intel-gfx] [PATCH 2/2] drm/i915: Add current GPU freq to sysfs

[Daniel Vetter]

On Sun, Sep 02, 2012 at 12:24:41AM -0700, 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
> 
> CC: Arjan van de Ven <arjan at linux.intel.com>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>

I've just noticed that your sloppy maintainer totally missed to pick up
Jesse's gt power consumption interface:

http://lists.freedesktop.org/archives/intel-gfx/2012-June/018404.html

Hence a bikeshed for the sysfs filename:
- render_ prefix is not accurate, this is for all of gt. I hence vote for
  a gt_
- I think calling it gt_cur_freq would make sense in case we'll expose
  _min/_max limits through sysfs, too.
- Also, calling it _cur_freq withouth MHz keeps in style with the
  frequency knobs exposed by cpus ...

Now I guess I should go back to my trace point patch and adjust it to
expose plain Hz, too ... Anyone got some bikeshed on this?
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_sysfs.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index da733a3..0cb479d 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -203,6 +203,30 @@ static struct bin_attribute dpf_attrs = {
>  	.mmap = NULL
>  };
>  
> +static ssize_t render_frequency_mhz_show(struct device *dev,
> +				     struct device_attribute *attr, char *buf)
> +{
> +	struct drm_minor *dminor = container_of(dev, struct drm_minor, kdev);
> +	struct drm_device *drm_dev = dminor->dev;
> +	struct drm_i915_private *dev_priv = drm_dev->dev_private;
> +	int ret;
> +
> +	ret = i915_mutex_lock_interruptible(drm_dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = dev_priv->rps.cur_delay * 50;
> +	mutex_unlock(&drm_dev->struct_mutex);
> +
> +	return snprintf(buf, PAGE_SIZE, "%d", ret);
> +}
> +
> +static struct device_attribute gen6_attrs[] = {
> +	__ATTR_RO(render_frequency_mhz),
> +	__ATTR_NULL,
> +};
> +
> +
>  void i915_setup_sysfs(struct drm_device *dev)
>  {
>  	int ret;
> @@ -220,10 +244,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 = device_create_file(&dev->primary->kdev, gen6_attrs);
> +		if (ret)
> +			DRM_ERROR("gen6 sysfs setup failed\n");
> +	}
>  }
>  
>  void i915_teardown_sysfs(struct drm_device *dev)
>  {
> +	device_remove_file(&dev->primary->kdev, gen6_attrs);
>  	device_remove_bin_file(&dev->primary->kdev,  &dpf_attrs);
>  	sysfs_unmerge_group(&dev->primary->kdev.kobj, &rc6_attr_group);
>  }
> -- 
> 1.7.12
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48