Re: [PATCH 3/3] drm/i915: userspace interface to the forcewake

[Paul Menzel <paulepanter@users.sourceforge.net>]

[-- Attachment #1.1: Type: text/plain, Size: 2527 bytes --]

Dear Ben,


Am Donnerstag, den 14.04.2011, 11:13 -0700 schrieb Ben Widawsky:
> userspace to the forcewake reference count via debugfs.
> 
> v2:
> use new spin_locks instead of struct_mutex

in my opinion these remarks should not go into the commit message.
Reading the commit log the reader is not interested in what patch
iteration some change was introduced.

> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---

Reviewing patches the comment, what changed in what iteration, is useful
though. So it should go after the `---` and be followed by a `---`, so
that `git am` ignores it.

        commit message
        
        S-o-b: Joe User <j@example.org>
        ---
        v2: changed foo
        ---
        diff

>  drivers/gpu/drm/i915/i915_debugfs.c |   77 +++++++++++++++++++++++++++++++++++
>  1 files changed, 77 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index cc3818f..fed41b9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1181,6 +1181,17 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
>  	return 0;
>  }
>  
> +static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
> +{
> +	struct drm_info_node *node = (struct drm_info_node *) m->private;
> +	struct drm_device *dev = node->minor->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	seq_printf(m, "forcewake count = %d\n", dev_priv->forcewake_count);
> +
> +	return 0;
> +}
> +
>  static int
>  i915_wedged_open(struct inode *inode,
>  		 struct file *filp)
> @@ -1283,6 +1294,65 @@ static int i915_wedged_create(struct dentry *root, struct drm_minor *minor)
>  	return drm_add_fake_info_node(minor, ent, &i915_wedged_fops);
>  }
>  
> +static int i915_forcewake_open(struct inode *inode, struct file *file)
> +{
> +	struct drm_device *dev = inode->i_private;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (!IS_GEN6(dev))
> +		return 0;
> +
> +	/*
> +	 * Paranoid people might say that it's bad that we can potentially hang
> +	 * userspace spinning on this lock. However, if we cannot acquire this
> +	 * lock it means that almost certainly the driver has hung, is not
> +	 * unload-able, and the GPU is not resettable. Therefore hanging here is
> +	 * probably a minor inconvenience not to be seen my almost every user.

s/my/by/

[…]


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx