[Intel-gfx] [PATCH 3/4] drm/i915: userspace interface to the forcewake refcount

Ben Widawsky ben at bwidawsk.net
Tue Apr 12 03:01:18 CEST 2011


Provide debugfs access to the refcounted forcewake. This allows a
userspace utility to access some of those hard to reach registers on
newer GPUs. The interface is root-only.

The interface is referred to as a forcewake lock. The term lock refers
to the synchronization between software and hardware (powering down the
GPU).

A note for those not reading the comments in the patches:
* "acquiring" the forcewake lock from userspace can fail. The utilities
  in userspace to read and write registers are inferior to the utilities
  in the kernel, and so failure (and inability to force acquire the
  lock) is acceptable.
* "releasing" the forcewake lock can hang. The conditions under which
  this can happen are so severe it would almost certainly require a
  reboot anyway (i915 unload would also hang).

Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   84 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.c     |   12 +++++
 2 files changed, 96 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2edb8b2..dda687a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1192,6 +1192,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)
@@ -1294,6 +1305,72 @@ 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;
+	int ret;
+
+	if (!IS_GEN6(dev))
+		return 0;
+
+	/*
+	 * If the driver hangs while holding the lock, the user should turn on
+	 * register read/write tracing, or use the other available debug options
+	 * to to find the cause.
+	 */
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	gen6_gt_force_wake_get(dev_priv);
+
+	mutex_unlock(&dev->struct_mutex);
+
+	return 0;
+}
+
+int i915_forcewake_release(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;
+
+	/*
+	 * If we hang, it implies that we have a locking issue, or some other
+	 * oops in the driver. If we lock up userspace, that is an unfortunate
+	 * but acceptable consequence.
+	 */
+	mutex_lock(&dev->struct_mutex);
+	gen6_gt_force_wake_put(dev_priv);
+	mutex_unlock(&dev->struct_mutex);
+
+	return 0;
+}
+
+static const struct file_operations i915_forcewake_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_forcewake_open,
+	.release = i915_forcewake_release,
+};
+
+static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor)
+{
+	struct drm_device *dev = minor->dev;
+	struct dentry *ent;
+
+	ent = debugfs_create_file("i915_forcewake_lock",
+				  S_IRWXU,
+				  root, dev,
+				  &i915_forcewake_fops);
+	if (IS_ERR(ent))
+		return PTR_ERR(ent);
+
+	return 0;
+}
+
 static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_capabilities", i915_capabilities, 0},
 	{"i915_gem_objects", i915_gem_object_info, 0},
@@ -1330,6 +1407,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_sr_status", i915_sr_status, 0},
 	{"i915_opregion", i915_opregion, 0},
 	{"i915_gem_framebuffer", i915_gem_framebuffer_info, 0},
+	{"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
@@ -1341,6 +1419,10 @@ int i915_debugfs_init(struct drm_minor *minor)
 	if (ret)
 		return ret;
 
+	ret = i915_forcewake_create(minor->debugfs_root, minor);
+	if (ret)
+		return ret;
+
 	return drm_debugfs_create_files(i915_debugfs_list,
 					I915_DEBUGFS_ENTRIES,
 					minor->debugfs_root, minor);
@@ -1350,6 +1432,8 @@ void i915_debugfs_cleanup(struct drm_minor *minor)
 {
 	drm_debugfs_remove_files(i915_debugfs_list,
 				 I915_DEBUGFS_ENTRIES, minor);
+	drm_debugfs_remove_files((struct drm_info_list *) &i915_forcewake_fops,
+				 1, minor);
 	drm_debugfs_remove_files((struct drm_info_list *) &i915_wedged_fops,
 				 1, minor);
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ce59cd4..0894e9f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -281,6 +281,15 @@ static void __gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 
 void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
 {
+	/*
+	 * struct_mutex protects the reference count. It must be held here. This
+	 * means that any register read which requires forcewake must hold
+	 * struct_mutex. Register reads which were previously protected by
+	 * config.mutex that require forcewake must now also hold struct_mutex.
+	 *
+	 * The other option is to introduce a new forcewake lock which must be
+	 * acquired prior to any register read.
+	 */
 	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
 
 	if (dev_priv->forcewake_count++ == 0)
@@ -295,6 +304,9 @@ static void __gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 
 void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
 {
+	/*
+	 * See gen6_gt_force_wake_get()
+	 */
 	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
 
 	if (--dev_priv->forcewake_count == 0)
-- 
1.7.3.4




More information about the Intel-gfx mailing list