Re: [PATCH] drm/i915: Add debugfs entry to read/write next_seqno

[Chris Wilson <chris@chris-wilson.co.uk>]

On Tue, 13 Nov 2012 15:48:28 +0200, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> This is needed for testing seqno wrapping. Be careful to not bump next_seqno
> more than 0x7FFFFFFF at a time (between some handled requests) as
> i915_seqno_passed() can't handle bigger difference in between.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |   83 +++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 4568e7d..8da4f55 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -839,6 +839,83 @@ 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)
> +{
> +	struct drm_device *dev = filp->private_data;
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	char buf[80];
> +	int len;
> +
> +	len = snprintf(buf, sizeof(buf),
> +		       "next_seqno :  0x%x\n",
> +		       dev_priv->next_seqno);

Probably worth the overhead of taking the mutex here for consistency.
Saves actual having to think in future.

> +
> +	if (len > sizeof(buf))
> +		len = sizeof(buf);
> +
> +	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +}
> +
> +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;
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	char buf[20];
> +	u32 val = 1;
> +	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;
> +	}
> +
> +	if (val == 0)
> +		return -EINVAL;
> +	ret = mutex_lock_interruptible(&dev->struct_mutex);
> +	if (ret)
> +		return ret;
> +
> +	if (val > dev_priv->next_seqno) {
i915_seqno_passed() ?

> +		dev_priv->next_seqno = val;
> +		ret = 0;
redundant

> +		DRM_DEBUG_DRIVER("Advancing seqno to %u\n", val);
> +	} else {
> +		ret = -EINVAL;
> +	}
> +
> +	mutex_unlock(&dev->struct_mutex);
> +
> +	if (ret)
> +		return ret;
> +
> +	return cnt;
return ret ?: cnt;

> +}
> +
> +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,
> +};
> +
>  static int i915_rstdby_delays(struct seq_file *m, void *unused)
>  {
>  	struct drm_info_node *node = (struct drm_info_node *) m->private;
> @@ -2103,6 +2180,12 @@ int i915_debugfs_init(struct drm_minor *minor)
>  	if (ret)
>  		return ret;
>  
> +	ret = i915_debugfs_create(minor->debugfs_root, minor,
> +				 "i915_next_seqno",
> +				 &i915_next_seqno_fops);
> +	if (ret)
> +		return ret;

We really need to start cleaning up these files along an error path...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre