diff mbox

[6/7,v3] drm/i915: Add setters for min/max frequency

Message ID 1347072225-10654-7-git-send-email-ben@bwidawsk.net (mailing list archive)
State Changes Requested
Headers show

Commit Message

Ben Widawsky Sept. 8, 2012, 2:43 a.m. UTC 
Provide a standardized sysfs interface for setting min, and max
frequencies.  The code which reads the limits were lifted from the
debugfs files. As a brief explanation, the limits are similar to the CPU
p-states. We have 3 states:

RP0 - ie. max frequency
RP1 - ie. "preferred min" frequency
RPn - seriously lowest frequency

Initially Daniel asked me to clamp the writes to supported values, but
in conforming to the way the cpufreq drivers seem to work, instead
return -EINVAL (noticed by Jesse in discussion).
The values can be used by userspace wishing to control the limits of the
GPU (see the CC list for people who care).

v2 Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
v3: bug fix (Ben) -  was passing the MHz value to gen6_set_rps instead of
the step value. To fix, deal only with step values by doing the divide
at the top.

v2: add the dropped mutex_unlock in error cases (Chris)
EINVAL on both too min, or too max (Daniel)
---
 drivers/gpu/drm/i915/i915_sysfs.c | 88 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 86 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Sept. 12, 2012, 2:46 p.m. UTC | #1 
On Fri, Sep 07, 2012 at 07:43:43PM -0700, Ben Widawsky wrote:
> Provide a standardized sysfs interface for setting min, and max
> frequencies.  The code which reads the limits were lifted from the
> debugfs files. As a brief explanation, the limits are similar to the CPU
> p-states. We have 3 states:
> 
> RP0 - ie. max frequency
> RP1 - ie. "preferred min" frequency
> RPn - seriously lowest frequency
> 
> Initially Daniel asked me to clamp the writes to supported values, but
> in conforming to the way the cpufreq drivers seem to work, instead
> return -EINVAL (noticed by Jesse in discussion).
> The values can be used by userspace wishing to control the limits of the
> GPU (see the CC list for people who care).
> 
> v2 Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
> v3: bug fix (Ben) -  was passing the MHz value to gen6_set_rps instead of
> the step value. To fix, deal only with step values by doing the divide
> at the top.
> 
> v2: add the dropped mutex_unlock in error cases (Chris)
> EINVAL on both too min, or too max (Daniel)
> ---
>  drivers/gpu/drm/i915/i915_sysfs.c | 88 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index 1da07e3..b67f5d7 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -238,6 +238,48 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
>  	return snprintf(buf, PAGE_SIZE, "%d", ret);
>  }
>  
> +static ssize_t gt_max_freq_mhz_store(struct device *kdev,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t count)
> +{
> +	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;
> +	u32 val, rp_state_cap, hw_max, hw_min;
> +	ssize_t ret;
> +
> +	ret = kstrtou32(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	val /= GT_FREQUENCY_MULTIPLIER;
> +
> +	ret = mutex_lock_interruptible(&dev->struct_mutex);
> +	if (ret)
> +		return ret;
> +
> +	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> +	hw_max = (rp_state_cap & 0xff);
> +	hw_min = ((rp_state_cap & 0xff0000) >> 16);
> +
> +	if (val < hw_min || val > hw_max) {
> +		mutex_unlock(&dev->struct_mutex);
> +		return -EINVAL;
> +	}
> +
> +	if (val < dev_priv->rps.min_delay)
> +		val = dev_priv->rps.min_delay;

Just forwarding an irc discussion so I don't forget about it:
I think we need return -EINVAL here, too. Otherwise userspace gets an
-EINVAL for hw limits, but if it tries to set up an otherwise illegal
min/max pair we silently fix it up. Same applies for the min delay below.
-Daniel

> +
> +	if (dev_priv->rps.cur_delay > val)
> +		gen6_set_rps(dev_priv->dev, val);
> +
> +	dev_priv->rps.max_delay = val;
> +
> +	mutex_unlock(&dev->struct_mutex);
> +
> +	return count;
> +}
> +
>  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);
> @@ -255,9 +297,51 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute
>  	return snprintf(buf, PAGE_SIZE, "%d", ret);
>  }
>  
> +static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t count)
> +{
> +	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;
> +	u32 val, rp_state_cap, hw_max, hw_min;
> +	ssize_t ret;
> +
> +	ret = kstrtou32(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	val /= GT_FREQUENCY_MULTIPLIER;
> +
> +	ret = mutex_lock_interruptible(&dev->struct_mutex);
> +	if (ret)
> +		return ret;
> +
> +	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> +	hw_max = (rp_state_cap & 0xff);
> +	hw_min = ((rp_state_cap & 0xff0000) >> 16);
> +
> +	if (val < hw_min || val > hw_max) {
> +		mutex_unlock(&dev->struct_mutex);
> +		return -EINVAL;
> +	}
> +
> +	if (val > dev_priv->rps.max_delay)
> +		val = dev_priv->rps.max_delay;
> +
> +	if (dev_priv->rps.cur_delay < val)
> +		gen6_set_rps(dev_priv->dev, val);
> +
> +	dev_priv->rps.min_delay = val;
> +
> +	mutex_unlock(&dev->struct_mutex);
> +
> +	return count;
> +}
> +
>  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 DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, gt_max_freq_mhz_store);
> +static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, gt_min_freq_mhz_store);
>  
>  static const struct attribute *gen6_attrs[] = {
>  	&dev_attr_gt_cur_freq_mhz.attr,
> -- 
> 1.7.12
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 1da07e3..b67f5d7 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -238,6 +238,48 @@  static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute
 	return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_max_freq_mhz_store(struct device *kdev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	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;
+	u32 val, rp_state_cap, hw_max, hw_min;
+	ssize_t ret;
+
+	ret = kstrtou32(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	val /= GT_FREQUENCY_MULTIPLIER;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+	hw_max = (rp_state_cap & 0xff);
+	hw_min = ((rp_state_cap & 0xff0000) >> 16);
+
+	if (val < hw_min || val > hw_max) {
+		mutex_unlock(&dev->struct_mutex);
+		return -EINVAL;
+	}
+
+	if (val < dev_priv->rps.min_delay)
+		val = dev_priv->rps.min_delay;
+
+	if (dev_priv->rps.cur_delay > val)
+		gen6_set_rps(dev_priv->dev, val);
+
+	dev_priv->rps.max_delay = val;
+
+	mutex_unlock(&dev->struct_mutex);
+
+	return count;
+}
+
 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);
@@ -255,9 +297,51 @@  static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute
 	return snprintf(buf, PAGE_SIZE, "%d", ret);
 }
 
+static ssize_t gt_min_freq_mhz_store(struct device *kdev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	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;
+	u32 val, rp_state_cap, hw_max, hw_min;
+	ssize_t ret;
+
+	ret = kstrtou32(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	val /= GT_FREQUENCY_MULTIPLIER;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+	hw_max = (rp_state_cap & 0xff);
+	hw_min = ((rp_state_cap & 0xff0000) >> 16);
+
+	if (val < hw_min || val > hw_max) {
+		mutex_unlock(&dev->struct_mutex);
+		return -EINVAL;
+	}
+
+	if (val > dev_priv->rps.max_delay)
+		val = dev_priv->rps.max_delay;
+
+	if (dev_priv->rps.cur_delay < val)
+		gen6_set_rps(dev_priv->dev, val);
+
+	dev_priv->rps.min_delay = val;
+
+	mutex_unlock(&dev->struct_mutex);
+
+	return count;
+}
+
 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 DEVICE_ATTR(gt_max_freq_mhz, S_IRUGO | S_IWUSR, gt_max_freq_mhz_show, gt_max_freq_mhz_store);
+static DEVICE_ATTR(gt_min_freq_mhz, S_IRUGO | S_IWUSR, gt_min_freq_mhz_show, gt_min_freq_mhz_store);
 
 static const struct attribute *gen6_attrs[] = {
 	&dev_attr_gt_cur_freq_mhz.attr,