diff mbox

drm/i915: use simple attribute in debugfs routines

Message ID 20130310211006.GA26200@www.outflux.net (mailing list archive)
State New, archived
Headers show

Commit Message

Kees Cook March 10, 2013, 9:10 p.m. UTC 
This replaces the manual read/write routines in debugfs with the common
simple attribute helpers. Doing this gets rid of repeated copy/pasting
of copy_from_user and value formatting code.

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  394 +++++++++--------------------------
 1 file changed, 98 insertions(+), 296 deletions(-)

Comments

Daniel Vetter March 11, 2013, 11:03 p.m. UTC | #1 
On Sun, Mar 10, 2013 at 02:10:06PM -0700, Kees Cook wrote:
> This replaces the manual read/write routines in debugfs with the common
> simple attribute helpers. Doing this gets rid of repeated copy/pasting
> of copy_from_user and value formatting code.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Queued for -next, thanks for the patch. When sending drm/i915 patches,
please also cc intel-gfx@lists.freedesktop.org (it's open for
non-subscribers non without nagging "you're mail is in the moderation
queue" replies).

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |  394 +++++++++--------------------------
>  1 file changed, 98 insertions(+), 296 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index aae3148..d86c304 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -849,76 +849,42 @@ static const struct file_operations i915_error_state_fops = {
>  	.release = i915_error_state_release,
>  };
>  
> -static ssize_t
> -i915_next_seqno_read(struct file *filp,
> -		 char __user *ubuf,
> -		 size_t max,
> -		 loff_t *ppos)
> +static int
> +i915_next_seqno_get(void *data, u64 *val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	char buf[80];
> -	int len;
>  	int ret;
>  
>  	ret = mutex_lock_interruptible(&dev->struct_mutex);
>  	if (ret)
>  		return ret;
>  
> -	len = snprintf(buf, sizeof(buf),
> -		       "next_seqno :  0x%x\n",
> -		       dev_priv->next_seqno);
> -
> +	*val = dev_priv->next_seqno;
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	if (len > sizeof(buf))
> -		len = sizeof(buf);
> -
> -	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +	return 0;
>  }
>  
> -static ssize_t
> -i915_next_seqno_write(struct file *filp,
> -		      const char __user *ubuf,
> -		      size_t cnt,
> -		      loff_t *ppos)
> -{
> -	struct drm_device *dev = filp->private_data;
> -	char buf[20];
> -	u32 val = 1;
> +static int
> +i915_next_seqno_set(void *data, u64 val)
> +{
> +	struct drm_device *dev = data;
>  	int ret;
>  
> -	if (cnt > 0) {
> -		if (cnt > sizeof(buf) - 1)
> -			return -EINVAL;
> -
> -		if (copy_from_user(buf, ubuf, cnt))
> -			return -EFAULT;
> -		buf[cnt] = 0;
> -
> -		ret = kstrtouint(buf, 0, &val);
> -		if (ret < 0)
> -			return ret;
> -	}
> -
>  	ret = mutex_lock_interruptible(&dev->struct_mutex);
>  	if (ret)
>  		return ret;
>  
>  	ret = i915_gem_set_seqno(dev, val);
> -
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	return ret ?: cnt;
> +	return ret;
>  }
>  
> -static const struct file_operations i915_next_seqno_fops = {
> -	.owner = THIS_MODULE,
> -	.open = simple_open,
> -	.read = i915_next_seqno_read,
> -	.write = i915_next_seqno_write,
> -	.llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_next_seqno_fops,
> +			i915_next_seqno_get, i915_next_seqno_set,
> +			"next_seqno :  0x%llx\n");
>  
>  static int i915_rstdby_delays(struct seq_file *m, void *unused)
>  {
> @@ -1680,105 +1646,51 @@ static int i915_dpio_info(struct seq_file *m, void *data)
>  	return 0;
>  }
>  
> -static ssize_t
> -i915_wedged_read(struct file *filp,
> -		 char __user *ubuf,
> -		 size_t max,
> -		 loff_t *ppos)
> +static int
> +i915_wedged_get(void *data, u64 *val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	char buf[80];
> -	int len;
>  
> -	len = snprintf(buf, sizeof(buf),
> -		       "wedged :  %d\n",
> -		       atomic_read(&dev_priv->gpu_error.reset_counter));
> +	*val = atomic_read(&dev_priv->gpu_error.reset_counter);
>  
> -	if (len > sizeof(buf))
> -		len = sizeof(buf);
> -
> -	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +	return 0;
>  }
>  
> -static ssize_t
> -i915_wedged_write(struct file *filp,
> -		  const char __user *ubuf,
> -		  size_t cnt,
> -		  loff_t *ppos)
> +static int
> +i915_wedged_set(void *data, u64 val)
>  {
> -	struct drm_device *dev = filp->private_data;
> -	char buf[20];
> -	int val = 1;
> -
> -	if (cnt > 0) {
> -		if (cnt > sizeof(buf) - 1)
> -			return -EINVAL;
> +	struct drm_device *dev = data;
>  
> -		if (copy_from_user(buf, ubuf, cnt))
> -			return -EFAULT;
> -		buf[cnt] = 0;
> -
> -		val = simple_strtoul(buf, NULL, 0);
> -	}
> -
> -	DRM_INFO("Manually setting wedged to %d\n", val);
> +	DRM_INFO("Manually setting wedged to %llu\n", val);
>  	i915_handle_error(dev, val);
>  
> -	return cnt;
> +	return 0;
>  }
>  
> -static const struct file_operations i915_wedged_fops = {
> -	.owner = THIS_MODULE,
> -	.open = simple_open,
> -	.read = i915_wedged_read,
> -	.write = i915_wedged_write,
> -	.llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops,
> +			i915_wedged_get, i915_wedged_set,
> +			"wedged :  %llu\n");
>  
> -static ssize_t
> -i915_ring_stop_read(struct file *filp,
> -		    char __user *ubuf,
> -		    size_t max,
> -		    loff_t *ppos)
> +static int
> +i915_ring_stop_get(void *data, u64 *val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	char buf[20];
> -	int len;
> -
> -	len = snprintf(buf, sizeof(buf),
> -		       "0x%08x\n", dev_priv->gpu_error.stop_rings);
>  
> -	if (len > sizeof(buf))
> -		len = sizeof(buf);
> +	*val = dev_priv->gpu_error.stop_rings;
>  
> -	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +	return 0;
>  }
>  
> -static ssize_t
> -i915_ring_stop_write(struct file *filp,
> -		     const char __user *ubuf,
> -		     size_t cnt,
> -		     loff_t *ppos)
> +static int
> +i915_ring_stop_set(void *data, u64 val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	char buf[20];
> -	int val = 0, ret;
> -
> -	if (cnt > 0) {
> -		if (cnt > sizeof(buf) - 1)
> -			return -EINVAL;
> -
> -		if (copy_from_user(buf, ubuf, cnt))
> -			return -EFAULT;
> -		buf[cnt] = 0;
> -
> -		val = simple_strtoul(buf, NULL, 0);
> -	}
> +	int ret;
>  
> -	DRM_DEBUG_DRIVER("Stopping rings 0x%08x\n", val);
> +	DRM_DEBUG_DRIVER("Stopping rings 0x%08llx\n", val);
>  
>  	ret = mutex_lock_interruptible(&dev->struct_mutex);
>  	if (ret)
> @@ -1787,16 +1699,12 @@ i915_ring_stop_write(struct file *filp,
>  	dev_priv->gpu_error.stop_rings = val;
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	return cnt;
> +	return 0;
>  }
>  
> -static const struct file_operations i915_ring_stop_fops = {
> -	.owner = THIS_MODULE,
> -	.open = simple_open,
> -	.read = i915_ring_stop_read,
> -	.write = i915_ring_stop_write,
> -	.llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_ring_stop_fops,
> +			i915_ring_stop_get, i915_ring_stop_set,
> +			"0x%08llx\n");
>  
>  #define DROP_UNBOUND 0x1
>  #define DROP_BOUND 0x2
> @@ -1806,46 +1714,23 @@ static const struct file_operations i915_ring_stop_fops = {
>  		  DROP_BOUND | \
>  		  DROP_RETIRE | \
>  		  DROP_ACTIVE)
> -static ssize_t
> -i915_drop_caches_read(struct file *filp,
> -		      char __user *ubuf,
> -		      size_t max,
> -		      loff_t *ppos)
> +static int
> +i915_drop_caches_get(void *data, u64 *val)
>  {
> -	char buf[20];
> -	int len;
> -
> -	len = snprintf(buf, sizeof(buf), "0x%08x\n", DROP_ALL);
> -	if (len > sizeof(buf))
> -		len = sizeof(buf);
> +	*val = DROP_ALL;
>  
> -	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +	return 0;
>  }
>  
> -static ssize_t
> -i915_drop_caches_write(struct file *filp,
> -		       const char __user *ubuf,
> -		       size_t cnt,
> -		       loff_t *ppos)
> +static int
> +i915_drop_caches_set(void *data, u64 val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_gem_object *obj, *next;
> -	char buf[20];
> -	int val = 0, ret;
> -
> -	if (cnt > 0) {
> -		if (cnt > sizeof(buf) - 1)
> -			return -EINVAL;
> -
> -		if (copy_from_user(buf, ubuf, cnt))
> -			return -EFAULT;
> -		buf[cnt] = 0;
> -
> -		val = simple_strtoul(buf, NULL, 0);
> -	}
> +	int ret;
>  
> -	DRM_DEBUG_DRIVER("Dropping caches: 0x%08x\n", val);
> +	DRM_DEBUG_DRIVER("Dropping caches: 0x%08llx\n", val);
>  
>  	/* No need to check and wait for gpu resets, only libdrm auto-restarts
>  	 * on ioctls on -EAGAIN. */
> @@ -1883,27 +1768,19 @@ i915_drop_caches_write(struct file *filp,
>  unlock:
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	return ret ?: cnt;
> +	return ret;
>  }
>  
> -static const struct file_operations i915_drop_caches_fops = {
> -	.owner = THIS_MODULE,
> -	.open = simple_open,
> -	.read = i915_drop_caches_read,
> -	.write = i915_drop_caches_write,
> -	.llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_drop_caches_fops,
> +			i915_drop_caches_get, i915_drop_caches_set,
> +			"0x%08llx\n");
>  
> -static ssize_t
> -i915_max_freq_read(struct file *filp,
> -		   char __user *ubuf,
> -		   size_t max,
> -		   loff_t *ppos)
> +static int
> +i915_max_freq_get(void *data, u64 *val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	char buf[80];
> -	int len, ret;
> +	int ret;
>  
>  	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
>  		return -ENODEV;
> @@ -1912,42 +1789,23 @@ i915_max_freq_read(struct file *filp,
>  	if (ret)
>  		return ret;
>  
> -	len = snprintf(buf, sizeof(buf),
> -		       "max freq: %d\n", dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER);
> +	*val = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	if (len > sizeof(buf))
> -		len = sizeof(buf);
> -
> -	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +	return 0;
>  }
>  
> -static ssize_t
> -i915_max_freq_write(struct file *filp,
> -		  const char __user *ubuf,
> -		  size_t cnt,
> -		  loff_t *ppos)
> +static int
> +i915_max_freq_set(void *data, u64 val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	char buf[20];
> -	int val = 1, ret;
> +	int ret;
>  
>  	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
>  		return -ENODEV;
>  
> -	if (cnt > 0) {
> -		if (cnt > sizeof(buf) - 1)
> -			return -EINVAL;
> -
> -		if (copy_from_user(buf, ubuf, cnt))
> -			return -EFAULT;
> -		buf[cnt] = 0;
> -
> -		val = simple_strtoul(buf, NULL, 0);
> -	}
> -
> -	DRM_DEBUG_DRIVER("Manually setting max freq to %d\n", val);
> +	DRM_DEBUG_DRIVER("Manually setting max freq to %llu\n", val);
>  
>  	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
>  	if (ret)
> @@ -1961,25 +1819,19 @@ i915_max_freq_write(struct file *filp,
>  	gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	return cnt;
> +	return 0;
>  }
>  
> -static const struct file_operations i915_max_freq_fops = {
> -	.owner = THIS_MODULE,
> -	.open = simple_open,
> -	.read = i915_max_freq_read,
> -	.write = i915_max_freq_write,
> -	.llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_max_freq_fops,
> +			i915_max_freq_get, i915_max_freq_set,
> +			"max freq: %llu\n");
>  
> -static ssize_t
> -i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
> -		   loff_t *ppos)
> +static int
> +i915_min_freq_get(void *data, u64 *val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	char buf[80];
> -	int len, ret;
> +	int ret;
>  
>  	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
>  		return -ENODEV;
> @@ -1988,40 +1840,23 @@ i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
>  	if (ret)
>  		return ret;
>  
> -	len = snprintf(buf, sizeof(buf),
> -		       "min freq: %d\n", dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER);
> +	*val = dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER;
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	if (len > sizeof(buf))
> -		len = sizeof(buf);
> -
> -	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +	return 0;
>  }
>  
> -static ssize_t
> -i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
> -		    loff_t *ppos)
> +static int
> +i915_min_freq_set(void *data, u64 val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	char buf[20];
> -	int val = 1, ret;
> +	int ret;
>  
>  	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
>  		return -ENODEV;
>  
> -	if (cnt > 0) {
> -		if (cnt > sizeof(buf) - 1)
> -			return -EINVAL;
> -
> -		if (copy_from_user(buf, ubuf, cnt))
> -			return -EFAULT;
> -		buf[cnt] = 0;
> -
> -		val = simple_strtoul(buf, NULL, 0);
> -	}
> -
> -	DRM_DEBUG_DRIVER("Manually setting min freq to %d\n", val);
> +	DRM_DEBUG_DRIVER("Manually setting min freq to %llu\n", val);
>  
>  	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
>  	if (ret)
> @@ -2035,28 +1870,20 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
>  	gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	return cnt;
> +	return 0;
>  }
>  
> -static const struct file_operations i915_min_freq_fops = {
> -	.owner = THIS_MODULE,
> -	.open = simple_open,
> -	.read = i915_min_freq_read,
> -	.write = i915_min_freq_write,
> -	.llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_min_freq_fops,
> +			i915_min_freq_get, i915_min_freq_set,
> +			"min freq: %llu\n");
>  
> -static ssize_t
> -i915_cache_sharing_read(struct file *filp,
> -		   char __user *ubuf,
> -		   size_t max,
> -		   loff_t *ppos)
> +static int
> +i915_cache_sharing_get(void *data, u64 *val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	drm_i915_private_t *dev_priv = dev->dev_private;
> -	char buf[80];
>  	u32 snpcr;
> -	int len, ret;
> +	int ret;
>  
>  	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
>  		return -ENODEV;
> @@ -2068,46 +1895,25 @@ i915_cache_sharing_read(struct file *filp,
>  	snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
>  	mutex_unlock(&dev_priv->dev->struct_mutex);
>  
> -	len = snprintf(buf, sizeof(buf),
> -		       "%d\n", (snpcr & GEN6_MBC_SNPCR_MASK) >>
> -		       GEN6_MBC_SNPCR_SHIFT);
> -
> -	if (len > sizeof(buf))
> -		len = sizeof(buf);
> +	*val = (snpcr & GEN6_MBC_SNPCR_MASK) >> GEN6_MBC_SNPCR_SHIFT;
>  
> -	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +	return 0;
>  }
>  
> -static ssize_t
> -i915_cache_sharing_write(struct file *filp,
> -		  const char __user *ubuf,
> -		  size_t cnt,
> -		  loff_t *ppos)
> +static int
> +i915_cache_sharing_set(void *data, u64 val)
>  {
> -	struct drm_device *dev = filp->private_data;
> +	struct drm_device *dev = data;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	char buf[20];
>  	u32 snpcr;
> -	int val = 1;
>  
>  	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
>  		return -ENODEV;
>  
> -	if (cnt > 0) {
> -		if (cnt > sizeof(buf) - 1)
> -			return -EINVAL;
> -
> -		if (copy_from_user(buf, ubuf, cnt))
> -			return -EFAULT;
> -		buf[cnt] = 0;
> -
> -		val = simple_strtoul(buf, NULL, 0);
> -	}
> -
> -	if (val < 0 || val > 3)
> +	if (val > 3)
>  		return -EINVAL;
>  
> -	DRM_DEBUG_DRIVER("Manually setting uncore sharing to %d\n", val);
> +	DRM_DEBUG_DRIVER("Manually setting uncore sharing to %llu\n", val);
>  
>  	/* Update the cache sharing policy here as well */
>  	snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
> @@ -2115,16 +1921,12 @@ i915_cache_sharing_write(struct file *filp,
>  	snpcr |= (val << GEN6_MBC_SNPCR_SHIFT);
>  	I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr);
>  
> -	return cnt;
> +	return 0;
>  }
>  
> -static const struct file_operations i915_cache_sharing_fops = {
> -	.owner = THIS_MODULE,
> -	.open = simple_open,
> -	.read = i915_cache_sharing_read,
> -	.write = i915_cache_sharing_write,
> -	.llseek = default_llseek,
> -};
> +DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
> +			i915_cache_sharing_get, i915_cache_sharing_set,
> +			"%llu\n");
>  
>  /* As the drm_debugfs_init() routines are called before dev->dev_private is
>   * allocated we need to hook into the minor for release. */
> -- 
> 1.7.9.5
> 
> 
> -- 
> Kees Cook
> Chrome OS Security
Kees Cook March 12, 2013, 12:20 a.m. UTC | #2 
On Mon, Mar 11, 2013 at 4:03 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sun, Mar 10, 2013 at 02:10:06PM -0700, Kees Cook wrote:
>> This replaces the manual read/write routines in debugfs with the common
>> simple attribute helpers. Doing this gets rid of repeated copy/pasting
>> of copy_from_user and value formatting code.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Queued for -next, thanks for the patch. When sending drm/i915 patches,
> please also cc intel-gfx@lists.freedesktop.org (it's open for
> non-subscribers non without nagging "you're mail is in the moderation
> queue" replies).

Ah-ha, sure.

Should MAINTAINERS be updated to reflect this? It got skipped as a
destination due to the "subscribers-only" suffix.

-Kees
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index aae3148..d86c304 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -849,76 +849,42 @@  static const struct file_operations i915_error_state_fops = {
 	.release = i915_error_state_release,
 };
 
-static ssize_t
-i915_next_seqno_read(struct file *filp,
-		 char __user *ubuf,
-		 size_t max,
-		 loff_t *ppos)
+static int
+i915_next_seqno_get(void *data, u64 *val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	char buf[80];
-	int len;
 	int ret;
 
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
 	if (ret)
 		return ret;
 
-	len = snprintf(buf, sizeof(buf),
-		       "next_seqno :  0x%x\n",
-		       dev_priv->next_seqno);
-
+	*val = dev_priv->next_seqno;
 	mutex_unlock(&dev->struct_mutex);
 
-	if (len > sizeof(buf))
-		len = sizeof(buf);
-
-	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+	return 0;
 }
 
-static ssize_t
-i915_next_seqno_write(struct file *filp,
-		      const char __user *ubuf,
-		      size_t cnt,
-		      loff_t *ppos)
-{
-	struct drm_device *dev = filp->private_data;
-	char buf[20];
-	u32 val = 1;
+static int
+i915_next_seqno_set(void *data, u64 val)
+{
+	struct drm_device *dev = data;
 	int ret;
 
-	if (cnt > 0) {
-		if (cnt > sizeof(buf) - 1)
-			return -EINVAL;
-
-		if (copy_from_user(buf, ubuf, cnt))
-			return -EFAULT;
-		buf[cnt] = 0;
-
-		ret = kstrtouint(buf, 0, &val);
-		if (ret < 0)
-			return ret;
-	}
-
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
 	if (ret)
 		return ret;
 
 	ret = i915_gem_set_seqno(dev, val);
-
 	mutex_unlock(&dev->struct_mutex);
 
-	return ret ?: cnt;
+	return ret;
 }
 
-static const struct file_operations i915_next_seqno_fops = {
-	.owner = THIS_MODULE,
-	.open = simple_open,
-	.read = i915_next_seqno_read,
-	.write = i915_next_seqno_write,
-	.llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_next_seqno_fops,
+			i915_next_seqno_get, i915_next_seqno_set,
+			"next_seqno :  0x%llx\n");
 
 static int i915_rstdby_delays(struct seq_file *m, void *unused)
 {
@@ -1680,105 +1646,51 @@  static int i915_dpio_info(struct seq_file *m, void *data)
 	return 0;
 }
 
-static ssize_t
-i915_wedged_read(struct file *filp,
-		 char __user *ubuf,
-		 size_t max,
-		 loff_t *ppos)
+static int
+i915_wedged_get(void *data, u64 *val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	char buf[80];
-	int len;
 
-	len = snprintf(buf, sizeof(buf),
-		       "wedged :  %d\n",
-		       atomic_read(&dev_priv->gpu_error.reset_counter));
+	*val = atomic_read(&dev_priv->gpu_error.reset_counter);
 
-	if (len > sizeof(buf))
-		len = sizeof(buf);
-
-	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+	return 0;
 }
 
-static ssize_t
-i915_wedged_write(struct file *filp,
-		  const char __user *ubuf,
-		  size_t cnt,
-		  loff_t *ppos)
+static int
+i915_wedged_set(void *data, u64 val)
 {
-	struct drm_device *dev = filp->private_data;
-	char buf[20];
-	int val = 1;
-
-	if (cnt > 0) {
-		if (cnt > sizeof(buf) - 1)
-			return -EINVAL;
+	struct drm_device *dev = data;
 
-		if (copy_from_user(buf, ubuf, cnt))
-			return -EFAULT;
-		buf[cnt] = 0;
-
-		val = simple_strtoul(buf, NULL, 0);
-	}
-
-	DRM_INFO("Manually setting wedged to %d\n", val);
+	DRM_INFO("Manually setting wedged to %llu\n", val);
 	i915_handle_error(dev, val);
 
-	return cnt;
+	return 0;
 }
 
-static const struct file_operations i915_wedged_fops = {
-	.owner = THIS_MODULE,
-	.open = simple_open,
-	.read = i915_wedged_read,
-	.write = i915_wedged_write,
-	.llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops,
+			i915_wedged_get, i915_wedged_set,
+			"wedged :  %llu\n");
 
-static ssize_t
-i915_ring_stop_read(struct file *filp,
-		    char __user *ubuf,
-		    size_t max,
-		    loff_t *ppos)
+static int
+i915_ring_stop_get(void *data, u64 *val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	char buf[20];
-	int len;
-
-	len = snprintf(buf, sizeof(buf),
-		       "0x%08x\n", dev_priv->gpu_error.stop_rings);
 
-	if (len > sizeof(buf))
-		len = sizeof(buf);
+	*val = dev_priv->gpu_error.stop_rings;
 
-	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+	return 0;
 }
 
-static ssize_t
-i915_ring_stop_write(struct file *filp,
-		     const char __user *ubuf,
-		     size_t cnt,
-		     loff_t *ppos)
+static int
+i915_ring_stop_set(void *data, u64 val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	char buf[20];
-	int val = 0, ret;
-
-	if (cnt > 0) {
-		if (cnt > sizeof(buf) - 1)
-			return -EINVAL;
-
-		if (copy_from_user(buf, ubuf, cnt))
-			return -EFAULT;
-		buf[cnt] = 0;
-
-		val = simple_strtoul(buf, NULL, 0);
-	}
+	int ret;
 
-	DRM_DEBUG_DRIVER("Stopping rings 0x%08x\n", val);
+	DRM_DEBUG_DRIVER("Stopping rings 0x%08llx\n", val);
 
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
 	if (ret)
@@ -1787,16 +1699,12 @@  i915_ring_stop_write(struct file *filp,
 	dev_priv->gpu_error.stop_rings = val;
 	mutex_unlock(&dev->struct_mutex);
 
-	return cnt;
+	return 0;
 }
 
-static const struct file_operations i915_ring_stop_fops = {
-	.owner = THIS_MODULE,
-	.open = simple_open,
-	.read = i915_ring_stop_read,
-	.write = i915_ring_stop_write,
-	.llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_ring_stop_fops,
+			i915_ring_stop_get, i915_ring_stop_set,
+			"0x%08llx\n");
 
 #define DROP_UNBOUND 0x1
 #define DROP_BOUND 0x2
@@ -1806,46 +1714,23 @@  static const struct file_operations i915_ring_stop_fops = {
 		  DROP_BOUND | \
 		  DROP_RETIRE | \
 		  DROP_ACTIVE)
-static ssize_t
-i915_drop_caches_read(struct file *filp,
-		      char __user *ubuf,
-		      size_t max,
-		      loff_t *ppos)
+static int
+i915_drop_caches_get(void *data, u64 *val)
 {
-	char buf[20];
-	int len;
-
-	len = snprintf(buf, sizeof(buf), "0x%08x\n", DROP_ALL);
-	if (len > sizeof(buf))
-		len = sizeof(buf);
+	*val = DROP_ALL;
 
-	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+	return 0;
 }
 
-static ssize_t
-i915_drop_caches_write(struct file *filp,
-		       const char __user *ubuf,
-		       size_t cnt,
-		       loff_t *ppos)
+static int
+i915_drop_caches_set(void *data, u64 val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj, *next;
-	char buf[20];
-	int val = 0, ret;
-
-	if (cnt > 0) {
-		if (cnt > sizeof(buf) - 1)
-			return -EINVAL;
-
-		if (copy_from_user(buf, ubuf, cnt))
-			return -EFAULT;
-		buf[cnt] = 0;
-
-		val = simple_strtoul(buf, NULL, 0);
-	}
+	int ret;
 
-	DRM_DEBUG_DRIVER("Dropping caches: 0x%08x\n", val);
+	DRM_DEBUG_DRIVER("Dropping caches: 0x%08llx\n", val);
 
 	/* No need to check and wait for gpu resets, only libdrm auto-restarts
 	 * on ioctls on -EAGAIN. */
@@ -1883,27 +1768,19 @@  i915_drop_caches_write(struct file *filp,
 unlock:
 	mutex_unlock(&dev->struct_mutex);
 
-	return ret ?: cnt;
+	return ret;
 }
 
-static const struct file_operations i915_drop_caches_fops = {
-	.owner = THIS_MODULE,
-	.open = simple_open,
-	.read = i915_drop_caches_read,
-	.write = i915_drop_caches_write,
-	.llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_drop_caches_fops,
+			i915_drop_caches_get, i915_drop_caches_set,
+			"0x%08llx\n");
 
-static ssize_t
-i915_max_freq_read(struct file *filp,
-		   char __user *ubuf,
-		   size_t max,
-		   loff_t *ppos)
+static int
+i915_max_freq_get(void *data, u64 *val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	char buf[80];
-	int len, ret;
+	int ret;
 
 	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
 		return -ENODEV;
@@ -1912,42 +1789,23 @@  i915_max_freq_read(struct file *filp,
 	if (ret)
 		return ret;
 
-	len = snprintf(buf, sizeof(buf),
-		       "max freq: %d\n", dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER);
+	*val = dev_priv->rps.max_delay * GT_FREQUENCY_MULTIPLIER;
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	if (len > sizeof(buf))
-		len = sizeof(buf);
-
-	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+	return 0;
 }
 
-static ssize_t
-i915_max_freq_write(struct file *filp,
-		  const char __user *ubuf,
-		  size_t cnt,
-		  loff_t *ppos)
+static int
+i915_max_freq_set(void *data, u64 val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	char buf[20];
-	int val = 1, ret;
+	int ret;
 
 	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
 		return -ENODEV;
 
-	if (cnt > 0) {
-		if (cnt > sizeof(buf) - 1)
-			return -EINVAL;
-
-		if (copy_from_user(buf, ubuf, cnt))
-			return -EFAULT;
-		buf[cnt] = 0;
-
-		val = simple_strtoul(buf, NULL, 0);
-	}
-
-	DRM_DEBUG_DRIVER("Manually setting max freq to %d\n", val);
+	DRM_DEBUG_DRIVER("Manually setting max freq to %llu\n", val);
 
 	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
 	if (ret)
@@ -1961,25 +1819,19 @@  i915_max_freq_write(struct file *filp,
 	gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	return cnt;
+	return 0;
 }
 
-static const struct file_operations i915_max_freq_fops = {
-	.owner = THIS_MODULE,
-	.open = simple_open,
-	.read = i915_max_freq_read,
-	.write = i915_max_freq_write,
-	.llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_max_freq_fops,
+			i915_max_freq_get, i915_max_freq_set,
+			"max freq: %llu\n");
 
-static ssize_t
-i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
-		   loff_t *ppos)
+static int
+i915_min_freq_get(void *data, u64 *val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	char buf[80];
-	int len, ret;
+	int ret;
 
 	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
 		return -ENODEV;
@@ -1988,40 +1840,23 @@  i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max,
 	if (ret)
 		return ret;
 
-	len = snprintf(buf, sizeof(buf),
-		       "min freq: %d\n", dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER);
+	*val = dev_priv->rps.min_delay * GT_FREQUENCY_MULTIPLIER;
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	if (len > sizeof(buf))
-		len = sizeof(buf);
-
-	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+	return 0;
 }
 
-static ssize_t
-i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
-		    loff_t *ppos)
+static int
+i915_min_freq_set(void *data, u64 val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	char buf[20];
-	int val = 1, ret;
+	int ret;
 
 	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
 		return -ENODEV;
 
-	if (cnt > 0) {
-		if (cnt > sizeof(buf) - 1)
-			return -EINVAL;
-
-		if (copy_from_user(buf, ubuf, cnt))
-			return -EFAULT;
-		buf[cnt] = 0;
-
-		val = simple_strtoul(buf, NULL, 0);
-	}
-
-	DRM_DEBUG_DRIVER("Manually setting min freq to %d\n", val);
+	DRM_DEBUG_DRIVER("Manually setting min freq to %llu\n", val);
 
 	ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
 	if (ret)
@@ -2035,28 +1870,20 @@  i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt,
 	gen6_set_rps(dev, val / GT_FREQUENCY_MULTIPLIER);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	return cnt;
+	return 0;
 }
 
-static const struct file_operations i915_min_freq_fops = {
-	.owner = THIS_MODULE,
-	.open = simple_open,
-	.read = i915_min_freq_read,
-	.write = i915_min_freq_write,
-	.llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_min_freq_fops,
+			i915_min_freq_get, i915_min_freq_set,
+			"min freq: %llu\n");
 
-static ssize_t
-i915_cache_sharing_read(struct file *filp,
-		   char __user *ubuf,
-		   size_t max,
-		   loff_t *ppos)
+static int
+i915_cache_sharing_get(void *data, u64 *val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	char buf[80];
 	u32 snpcr;
-	int len, ret;
+	int ret;
 
 	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
 		return -ENODEV;
@@ -2068,46 +1895,25 @@  i915_cache_sharing_read(struct file *filp,
 	snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
 	mutex_unlock(&dev_priv->dev->struct_mutex);
 
-	len = snprintf(buf, sizeof(buf),
-		       "%d\n", (snpcr & GEN6_MBC_SNPCR_MASK) >>
-		       GEN6_MBC_SNPCR_SHIFT);
-
-	if (len > sizeof(buf))
-		len = sizeof(buf);
+	*val = (snpcr & GEN6_MBC_SNPCR_MASK) >> GEN6_MBC_SNPCR_SHIFT;
 
-	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+	return 0;
 }
 
-static ssize_t
-i915_cache_sharing_write(struct file *filp,
-		  const char __user *ubuf,
-		  size_t cnt,
-		  loff_t *ppos)
+static int
+i915_cache_sharing_set(void *data, u64 val)
 {
-	struct drm_device *dev = filp->private_data;
+	struct drm_device *dev = data;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	char buf[20];
 	u32 snpcr;
-	int val = 1;
 
 	if (!(IS_GEN6(dev) || IS_GEN7(dev)))
 		return -ENODEV;
 
-	if (cnt > 0) {
-		if (cnt > sizeof(buf) - 1)
-			return -EINVAL;
-
-		if (copy_from_user(buf, ubuf, cnt))
-			return -EFAULT;
-		buf[cnt] = 0;
-
-		val = simple_strtoul(buf, NULL, 0);
-	}
-
-	if (val < 0 || val > 3)
+	if (val > 3)
 		return -EINVAL;
 
-	DRM_DEBUG_DRIVER("Manually setting uncore sharing to %d\n", val);
+	DRM_DEBUG_DRIVER("Manually setting uncore sharing to %llu\n", val);
 
 	/* Update the cache sharing policy here as well */
 	snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
@@ -2115,16 +1921,12 @@  i915_cache_sharing_write(struct file *filp,
 	snpcr |= (val << GEN6_MBC_SNPCR_SHIFT);
 	I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr);
 
-	return cnt;
+	return 0;
 }
 
-static const struct file_operations i915_cache_sharing_fops = {
-	.owner = THIS_MODULE,
-	.open = simple_open,
-	.read = i915_cache_sharing_read,
-	.write = i915_cache_sharing_write,
-	.llseek = default_llseek,
-};
+DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
+			i915_cache_sharing_get, i915_cache_sharing_set,
+			"%llu\n");
 
 /* As the drm_debugfs_init() routines are called before dev->dev_private is
  * allocated we need to hook into the minor for release. */