[PATCH 1/4] drm/i915: Add gpu hw state capture helper

[PATCH 1/4] drm/i915: Add gpu hw state capture helper

[Mika Kuoppala]

In the following commits, we want to capture the hw state
also without any errors. Carve out the helper out from error
capture parts.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 41 +++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index c4536e1..dcea1fa 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1265,28 +1265,17 @@ static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
 	error->suspend_count = dev_priv->suspend_count;
 }
 
-/**
- * i915_capture_error_state - capture an error record for later analysis
- * @dev: drm device
- *
- * Should be called when an error is detected (either a hang or an error
- * interrupt) to capture error state from the time of the error.  Fills
- * out a structure which becomes available in debugfs for user level tools
- * to pick up.
- */
-void i915_capture_error_state(struct drm_device *dev, bool wedged,
-			      const char *error_msg)
+static struct drm_i915_error_state *
+__i915_capture_gpu_state(struct drm_device *dev)
 {
-	static bool warned;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_error_state *error;
-	unsigned long flags;
 
 	/* Account for pipe specific data like PIPE*STAT */
 	error = kzalloc(sizeof(*error), GFP_ATOMIC);
 	if (!error) {
 		DRM_DEBUG_DRIVER("out of memory, not capturing error state\n");
-		return;
+		return NULL;
 	}
 
 	kref_init(&error->ref);
@@ -1302,6 +1291,30 @@ void i915_capture_error_state(struct drm_device *dev, bool wedged,
 	error->overlay = intel_overlay_capture_error_state(dev);
 	error->display = intel_display_capture_error_state(dev);
 
+	return error;
+}
+
+/**
+ * i915_capture_error_state - capture an error record for later analysis
+ * @dev: drm device
+ *
+ * Should be called when an error is detected (either a hang or an error
+ * interrupt) to capture error state from the time of the error.  Fills
+ * out a structure which becomes available in debugfs for user level tools
+ * to pick up.
+ */
+void i915_capture_error_state(struct drm_device *dev, bool wedged,
+			      const char *error_msg)
+{
+	static bool warned;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_error_state *error;
+	unsigned long flags;
+
+	error = __i915_capture_gpu_state(dev);
+	if (error == NULL)
+		return;
+
 	i915_error_capture_msg(dev, error, wedged, error_msg);
 	DRM_INFO("%s\n", error->error_msg);
 
-- 
1.9.1

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

[PATCH 2/4] drm/i915: Add i915_gpu_state debugfs entry

[Mika Kuoppala]

'i915_gpu_state' debugfs entry can be used to
capture the current gpu state. This is similar to
what one would get from 'i915_error_state' if gpu
error state would have been captured.

The motivation for this was to enhance our toolbox
so that we can direct bug reporters to do things like:

'grep -i suspend /sys/kernel/debug/dri/0/i915_gpu_state'

pre and postmortem to gain insight in triaging and
save ourselves from writing some new debugfs entries, when
the information is already in our error state.

v2: - use symmetrical put/get (Chris)
    - document the api (Daniel)
    - take a mutex when capturing

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   | 129 ++++++++++++++++++++++------------
 drivers/gpu/drm/i915/i915_drv.h       |   5 ++
 drivers/gpu/drm/i915/i915_gpu_error.c |  57 +++++++++++++--
 3 files changed, 141 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d0e445e..30f56f3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -888,58 +888,41 @@ static int i915_hws_info(struct seq_file *m, void *data)
 	return 0;
 }
 
-static ssize_t
-i915_error_state_write(struct file *filp,
-		       const char __user *ubuf,
-		       size_t cnt,
-		       loff_t *ppos)
-{
-	struct i915_error_state_file_priv *error_priv = filp->private_data;
-	struct drm_device *dev = error_priv->dev;
-	int ret;
-
-	DRM_DEBUG_DRIVER("Resetting error state\n");
-
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
-	if (ret)
-		return ret;
-
-	i915_destroy_error_state(dev);
-	mutex_unlock(&dev->struct_mutex);
-
-	return cnt;
-}
-
-static int i915_error_state_open(struct inode *inode, struct file *file)
+static int i915_gpu_state_open(struct inode *inode, struct file *file)
 {
 	struct drm_device *dev = inode->i_private;
-	struct i915_error_state_file_priv *error_priv;
+	struct i915_error_state_file_priv *state_priv;
+	int ret = 0;
 
-	error_priv = kzalloc(sizeof(*error_priv), GFP_KERNEL);
-	if (!error_priv)
+	state_priv = kzalloc(sizeof(*state_priv), GFP_KERNEL);
+	if (!state_priv)
 		return -ENOMEM;
 
-	error_priv->dev = dev;
-
-	i915_error_state_get(dev, error_priv);
-
-	file->private_data = error_priv;
+	state_priv->dev = dev;
 
-	return 0;
-}
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
 
-static int i915_error_state_release(struct inode *inode, struct file *file)
-{
-	struct i915_error_state_file_priv *error_priv = file->private_data;
+	ret = i915_gpu_idle(dev);
+	if (ret)
+		goto unlock;
 
-	i915_error_state_put(error_priv);
-	kfree(error_priv);
+	state_priv->error = i915_gpu_state_capture(dev);
+	if (state_priv->error == NULL) {
+		kfree(state_priv);
+		ret = -ENOMEM;
+		goto unlock;
+	}
 
-	return 0;
+	file->private_data = state_priv;
+unlock:
+	mutex_unlock(&dev->struct_mutex);
+	return ret;
 }
 
-static ssize_t i915_error_state_read(struct file *file, char __user *userbuf,
-				     size_t count, loff_t *pos)
+static ssize_t i915_gpu_state_read(struct file *file, char __user *userbuf,
+				   size_t count, loff_t *pos)
 {
 	struct i915_error_state_file_priv *error_priv = file->private_data;
 	struct drm_i915_error_state_buf error_str;
@@ -968,13 +951,72 @@ out:
 	return ret ?: ret_count;
 }
 
+static int i915_gpu_state_release(struct inode *inode, struct file *file)
+{
+	struct i915_error_state_file_priv *state_priv = file->private_data;
+
+	i915_gpu_state_put(state_priv->error);
+	kfree(state_priv);
+
+	return 0;
+}
+
+static const struct file_operations i915_gpu_state_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_gpu_state_open,
+	.read = i915_gpu_state_read,
+	.write = NULL,
+	.llseek = default_llseek,
+	.release = i915_gpu_state_release,
+};
+
+static int i915_error_state_open(struct inode *inode, struct file *file)
+{
+	struct drm_device *dev = inode->i_private;
+	struct i915_error_state_file_priv *error_priv;
+
+	error_priv = kzalloc(sizeof(*error_priv), GFP_KERNEL);
+	if (!error_priv)
+		return -ENOMEM;
+
+	error_priv->dev = dev;
+
+	i915_error_state_get(dev, error_priv);
+
+	file->private_data = error_priv;
+
+	return 0;
+}
+
+static ssize_t
+i915_error_state_write(struct file *filp,
+		       const char __user *ubuf,
+		       size_t cnt,
+		       loff_t *ppos)
+{
+	struct i915_error_state_file_priv *error_priv = filp->private_data;
+	struct drm_device *dev = error_priv->dev;
+	int ret;
+
+	DRM_DEBUG_DRIVER("Resetting error state\n");
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	i915_destroy_error_state(dev);
+	mutex_unlock(&dev->struct_mutex);
+
+	return cnt;
+}
+
 static const struct file_operations i915_error_state_fops = {
 	.owner = THIS_MODULE,
 	.open = i915_error_state_open,
-	.read = i915_error_state_read,
+	.read = i915_gpu_state_read,
 	.write = i915_error_state_write,
 	.llseek = default_llseek,
-	.release = i915_error_state_release,
+	.release = i915_gpu_state_release,
 };
 
 static int
@@ -4367,6 +4409,7 @@ static const struct i915_debugfs_files {
 	{"i915_ring_test_irq", &i915_ring_test_irq_fops},
 	{"i915_gem_drop_caches", &i915_drop_caches_fops},
 	{"i915_error_state", &i915_error_state_fops},
+	{"i915_gpu_state", &i915_gpu_state_fops},
 	{"i915_next_seqno", &i915_next_seqno_fops},
 	{"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
 	{"i915_pri_wm_latency", &i915_pri_wm_latency_fops},
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 11e85cb..ca487b4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2866,6 +2866,11 @@ static inline void i915_error_state_buf_release(
 {
 	kfree(eb->buf);
 }
+
+struct drm_i915_error_state * __must_check
+i915_gpu_state_capture(struct drm_device *dev);
+void i915_gpu_state_put(struct drm_i915_error_state *error);
+
 void i915_capture_error_state(struct drm_device *dev, bool wedge,
 			      const char *error_msg);
 void i915_error_state_get(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index dcea1fa..0cefeebc 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -540,7 +540,7 @@ static void i915_error_object_free(struct drm_i915_error_object *obj)
 	kfree(obj);
 }
 
-static void i915_error_state_free(struct kref *error_ref)
+static void i915_gpu_state_free(struct kref *error_ref)
 {
 	struct drm_i915_error_state *error = container_of(error_ref,
 							  typeof(*error), ref);
@@ -1266,7 +1266,7 @@ static void i915_capture_gen_state(struct drm_i915_private *dev_priv,
 }
 
 static struct drm_i915_error_state *
-__i915_capture_gpu_state(struct drm_device *dev)
+__i915_gpu_state_capture(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_error_state *error;
@@ -1295,6 +1295,49 @@ __i915_capture_gpu_state(struct drm_device *dev)
 }
 
 /**
+ * i915_gpu_state_capture - capture a gpu hardware state
+ * @dev: drm device
+ *
+ * This will capture the current gpu hw state, identical what
+ * i915_capture_error_state() does, but without any error condition.
+ * The returned state is reference counted and caller is responsible
+ * to release the state by calling i915_gpu_state_put().
+ */
+struct drm_i915_error_state *
+i915_gpu_state_capture(struct drm_device *dev)
+{
+	struct drm_i915_error_state *gpu_state;
+
+	gpu_state = __i915_gpu_state_capture(dev);
+	if (gpu_state == NULL)
+		return NULL;
+
+	scnprintf(gpu_state->error_msg, sizeof(gpu_state->error_msg),
+		  "GPU hw state snapshot\n");
+
+	return gpu_state;
+}
+
+static void i915_gpu_state_get(struct drm_i915_error_state *state)
+{
+	WARN_ON(!state);
+	kref_get(&state->ref);
+}
+
+/**
+ * i915_gpu_state_put - release a drm_i915_error_state
+ * @state: gpu state
+ *
+ * This will release the reference to the the drm_i915_error_state.
+ * Possibly freeing the state if the reference count reaches zero.
+ */
+void i915_gpu_state_put(struct drm_i915_error_state *state)
+{
+	if (state)
+		kref_put(&state->ref, i915_gpu_state_free);
+}
+
+/**
  * i915_capture_error_state - capture an error record for later analysis
  * @dev: drm device
  *
@@ -1311,7 +1354,7 @@ void i915_capture_error_state(struct drm_device *dev, bool wedged,
 	struct drm_i915_error_state *error;
 	unsigned long flags;
 
-	error = __i915_capture_gpu_state(dev);
+	error = __i915_gpu_state_capture(dev);
 	if (error == NULL)
 		return;
 
@@ -1326,7 +1369,7 @@ void i915_capture_error_state(struct drm_device *dev, bool wedged,
 	spin_unlock_irqrestore(&dev_priv->gpu_error.lock, flags);
 
 	if (error) {
-		i915_error_state_free(&error->ref);
+		i915_gpu_state_free(&error->ref);
 		return;
 	}
 
@@ -1348,7 +1391,7 @@ void i915_error_state_get(struct drm_device *dev,
 	spin_lock_irq(&dev_priv->gpu_error.lock);
 	error_priv->error = dev_priv->gpu_error.first_error;
 	if (error_priv->error)
-		kref_get(&error_priv->error->ref);
+		i915_gpu_state_get(error_priv->error);
 	spin_unlock_irq(&dev_priv->gpu_error.lock);
 
 }
@@ -1356,7 +1399,7 @@ void i915_error_state_get(struct drm_device *dev,
 void i915_error_state_put(struct i915_error_state_file_priv *error_priv)
 {
 	if (error_priv->error)
-		kref_put(&error_priv->error->ref, i915_error_state_free);
+		i915_gpu_state_put(error_priv->error);
 }
 
 void i915_destroy_error_state(struct drm_device *dev)
@@ -1370,7 +1413,7 @@ void i915_destroy_error_state(struct drm_device *dev)
 	spin_unlock_irq(&dev_priv->gpu_error.lock);
 
 	if (error)
-		kref_put(&error->ref, i915_error_state_free);
+		i915_gpu_state_put(error);
 }
 
 const char *i915_cache_level_str(struct drm_i915_private *i915, int type)
-- 
1.9.1

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

[PATCH 3/4] drm/i915: Add i915_gpu_state_unsafe debugfs entry

[Mika Kuoppala]

This is a similar to 'i915_gpu_state' but more dangerous
as it doesn't try to lock nor idle the gpu before grabbing
the state. But the obvious advantages are that one can
inspect the current running gpu state and also get a state
dump even if mutex is hold.

As we don't want a unsuspecting user to trip into this by
accident, there needs to be write into the debugfs node
before any unsafe capturing can take place.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 55 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h     |  4 +++
 2 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 30f56f3..5480cf5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -970,6 +970,60 @@ static const struct file_operations i915_gpu_state_fops = {
 	.release = i915_gpu_state_release,
 };
 
+static int i915_gpu_state_open_unsafe(struct inode *inode, struct file *file)
+{
+	struct drm_device *dev = inode->i_private;
+	struct i915_error_state_file_priv *state_priv;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	state_priv = kzalloc(sizeof(*state_priv), GFP_KERNEL);
+	if (!state_priv)
+		return -ENOMEM;
+
+	state_priv->dev = dev;
+
+	if (dev_priv->gpu_error.allow_unsafe_state_capture) {
+		state_priv->error = i915_gpu_state_capture(dev);
+		if (state_priv->error == NULL) {
+			kfree(state_priv);
+			return -ENOMEM;
+		}
+	}
+
+	file->private_data = state_priv;
+	return 0;
+}
+
+static ssize_t
+i915_gpu_state_write_unsafe(struct file *filp,
+			    const char __user *ubuf,
+			    size_t cnt,
+			    loff_t *ppos)
+{
+	struct i915_error_state_file_priv *error_priv = filp->private_data;
+	struct drm_device *dev = error_priv->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	dev_priv->gpu_error.allow_unsafe_state_capture = !
+		dev_priv->gpu_error.allow_unsafe_state_capture;
+
+	if (dev_priv->gpu_error.allow_unsafe_state_capture)
+		DRM_DEBUG_DRIVER("allowing unsafe, non locked gpu state dumps\n");
+	else
+		DRM_DEBUG_DRIVER("denying unsafe, non locked gpu state dumps\n");
+
+	return cnt;
+}
+
+static const struct file_operations i915_gpu_state_unsafe_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_gpu_state_open_unsafe,
+	.read = i915_gpu_state_read,
+	.write = i915_gpu_state_write_unsafe,
+	.llseek = default_llseek,
+	.release = i915_gpu_state_release,
+};
+
 static int i915_error_state_open(struct inode *inode, struct file *file)
 {
 	struct drm_device *dev = inode->i_private;
@@ -4410,6 +4464,7 @@ static const struct i915_debugfs_files {
 	{"i915_gem_drop_caches", &i915_drop_caches_fops},
 	{"i915_error_state", &i915_error_state_fops},
 	{"i915_gpu_state", &i915_gpu_state_fops},
+	{"i915_gpu_state_unsafe", &i915_gpu_state_unsafe_fops},
 	{"i915_next_seqno", &i915_next_seqno_fops},
 	{"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
 	{"i915_pri_wm_latency", &i915_pri_wm_latency_fops},
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ca487b4..452f93f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1278,6 +1278,10 @@ struct i915_gpu_error {
 
 	/* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset   */
 	bool reload_in_reset;
+
+	/* Safety mechanism to prevent unsuspecting user to obtain nonlocked
+	 * capture by accident */
+	bool allow_unsafe_state_capture;
 };
 
 enum modeset_restore {
-- 
1.9.1

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

[PATCH 4/4] drm/i915: Rename drm_i915_error_state_buf to intel_strbuf

[Mika Kuoppala]

Emphasize the more general applicability of the seekable
string buffer by decoupling it from error handling.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
 drivers/gpu/drm/i915/i915_drv.h       | 14 +++----
 drivers/gpu/drm/i915/i915_gpu_error.c | 74 ++++++++++++++++++++---------------
 drivers/gpu/drm/i915/i915_sysfs.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c  |  4 +-
 drivers/gpu/drm/i915/intel_overlay.c  |  8 ++--
 6 files changed, 58 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5480cf5..f9e3449 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -925,7 +925,7 @@ static ssize_t i915_gpu_state_read(struct file *file, char __user *userbuf,
 				   size_t count, loff_t *pos)
 {
 	struct i915_error_state_file_priv *error_priv = file->private_data;
-	struct drm_i915_error_state_buf error_str;
+	struct intel_strbuf error_str;
 	loff_t tmp_pos = 0;
 	ssize_t ret_count = 0;
 	int ret;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 452f93f..7ac3c71 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1201,7 +1201,7 @@ struct i915_gem_mm {
 	u32 object_count;
 };
 
-struct drm_i915_error_state_buf {
+struct intel_strbuf {
 	struct drm_i915_private *i915;
 	unsigned bytes;
 	unsigned size;
@@ -2859,14 +2859,14 @@ static inline void intel_display_crc_init(struct drm_device *dev) {}
 
 /* i915_gpu_error.c */
 __printf(2, 3)
-void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...);
-int i915_error_state_to_str(struct drm_i915_error_state_buf *estr,
+void intel_strbuf_printf(struct intel_strbuf *e, const char *f, ...);
+int i915_error_state_to_str(struct intel_strbuf *estr,
 			    const struct i915_error_state_file_priv *error);
-int i915_error_state_buf_init(struct drm_i915_error_state_buf *eb,
+int i915_error_state_buf_init(struct intel_strbuf *eb,
 			      struct drm_i915_private *i915,
 			      size_t count, loff_t pos);
 static inline void i915_error_state_buf_release(
-	struct drm_i915_error_state_buf *eb)
+	struct intel_strbuf *eb)
 {
 	kfree(eb->buf);
 }
@@ -2995,11 +2995,11 @@ void intel_notify_mmio_flip(struct intel_engine_cs *ring);
 
 /* overlay */
 extern struct intel_overlay_error_state *intel_overlay_capture_error_state(struct drm_device *dev);
-extern void intel_overlay_print_error_state(struct drm_i915_error_state_buf *e,
+extern void intel_overlay_print_error_state(struct intel_strbuf *e,
 					    struct intel_overlay_error_state *error);
 
 extern struct intel_display_error_state *intel_display_capture_error_state(struct drm_device *dev);
-extern void intel_display_print_error_state(struct drm_i915_error_state_buf *e,
+extern void intel_display_print_error_state(struct intel_strbuf *e,
 					    struct drm_device *dev,
 					    struct intel_display_error_state *error);
 
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0cefeebc..78c6d87 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -77,7 +77,7 @@ static const char *purgeable_flag(int purgeable)
 	return purgeable ? " purgeable" : "";
 }
 
-static bool __i915_error_ok(struct drm_i915_error_state_buf *e)
+static bool __intel_strbuf_ok(struct intel_strbuf *e)
 {
 
 	if (!e->err && WARN(e->bytes > (e->size - 1), "overflow")) {
@@ -91,8 +91,8 @@ static bool __i915_error_ok(struct drm_i915_error_state_buf *e)
 	return true;
 }
 
-static bool __i915_error_seek(struct drm_i915_error_state_buf *e,
-			      unsigned len)
+static bool __intel_strbuf_seek(struct intel_strbuf *e,
+				unsigned len)
 {
 	if (e->pos + len <= e->start) {
 		e->pos += len;
@@ -108,8 +108,8 @@ static bool __i915_error_seek(struct drm_i915_error_state_buf *e,
 	return true;
 }
 
-static void __i915_error_advance(struct drm_i915_error_state_buf *e,
-				 unsigned len)
+static void __intel_strbuf_advance(struct intel_strbuf *e,
+				   unsigned len)
 {
 	/* If this is first printf in this window, adjust it so that
 	 * start position matches start of the buffer
@@ -134,12 +134,12 @@ static void __i915_error_advance(struct drm_i915_error_state_buf *e,
 	e->pos += len;
 }
 
-static void i915_error_vprintf(struct drm_i915_error_state_buf *e,
-			       const char *f, va_list args)
+static void intel_strbuf_vprintf(struct intel_strbuf *e,
+				 const char *f, va_list args)
 {
 	unsigned len;
 
-	if (!__i915_error_ok(e))
+	if (!__intel_strbuf_ok(e))
 		return;
 
 	/* Seek the first printf which is hits start position */
@@ -150,7 +150,7 @@ static void i915_error_vprintf(struct drm_i915_error_state_buf *e,
 		len = vsnprintf(NULL, 0, f, tmp);
 		va_end(tmp);
 
-		if (!__i915_error_seek(e, len))
+		if (!__intel_strbuf_seek(e, len))
 			return;
 	}
 
@@ -158,22 +158,22 @@ static void i915_error_vprintf(struct drm_i915_error_state_buf *e,
 	if (len >= e->size - e->bytes)
 		len = e->size - e->bytes - 1;
 
-	__i915_error_advance(e, len);
+	__intel_strbuf_advance(e, len);
 }
 
-static void i915_error_puts(struct drm_i915_error_state_buf *e,
-			    const char *str)
+static void intel_strbuf_puts(struct intel_strbuf *e,
+			      const char *str)
 {
 	unsigned len;
 
-	if (!__i915_error_ok(e))
+	if (!__intel_strbuf_ok(e))
 		return;
 
 	len = strlen(str);
 
 	/* Seek the first printf which is hits start position */
 	if (e->pos < e->start) {
-		if (!__i915_error_seek(e, len))
+		if (!__intel_strbuf_seek(e, len))
 			return;
 	}
 
@@ -181,13 +181,34 @@ static void i915_error_puts(struct drm_i915_error_state_buf *e,
 		len = e->size - e->bytes - 1;
 	memcpy(e->buf + e->bytes, str, len);
 
-	__i915_error_advance(e, len);
+	__intel_strbuf_advance(e, len);
 }
 
-#define err_printf(e, ...) i915_error_printf(e, __VA_ARGS__)
-#define err_puts(e, s) i915_error_puts(e, s)
+/**
+ * intel_strbuf_printf - print formatted string into intel_strbuf
+ * @strbuf: destination string buffer
+ * @format: format string
+ * @...: variable arguments for each format specifier in @format.
+ *
+ * Print formatted string into intel_strbuf. struct intel_strbuf is
+ * a seekable string buffer. It is used to seek into a potentially
+ * very big string without the need to allocate the whole string
+ * beforehand. Only the seekable 'window' is allocated at a time.
+ * __intel_strbuf_ok() can be used to check for errors after this call.
+ */
+void intel_strbuf_printf(struct intel_strbuf *strbuf, const char *format, ...)
+{
+	va_list args;
+
+	va_start(args, format);
+	intel_strbuf_vprintf(strbuf, format, args);
+	va_end(args);
+}
 
-static void print_error_buffers(struct drm_i915_error_state_buf *m,
+#define err_printf(e, ...) intel_strbuf_printf(e, __VA_ARGS__)
+#define err_puts(e, s) intel_strbuf_puts(e, s)
+
+static void print_error_buffers(struct intel_strbuf *m,
 				const char *name,
 				struct drm_i915_error_buffer *err,
 				int count)
@@ -240,7 +261,7 @@ static const char *hangcheck_action_to_str(enum intel_ring_hangcheck_action a)
 	return "unknown";
 }
 
-static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
+static void i915_ring_error_state(struct intel_strbuf *m,
 				  struct drm_device *dev,
 				  struct drm_i915_error_state *error,
 				  int ring_idx)
@@ -304,16 +325,7 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
 		   ring->hangcheck_score);
 }
 
-void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...)
-{
-	va_list args;
-
-	va_start(args, f);
-	i915_error_vprintf(e, f, args);
-	va_end(args);
-}
-
-static void print_error_obj(struct drm_i915_error_state_buf *m,
+static void print_error_obj(struct intel_strbuf *m,
 			    struct drm_i915_error_object *obj)
 {
 	int page, offset, elt;
@@ -327,7 +339,7 @@ static void print_error_obj(struct drm_i915_error_state_buf *m,
 	}
 }
 
-int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
+int i915_error_state_to_str(struct intel_strbuf *m,
 			    const struct i915_error_state_file_priv *error_priv)
 {
 	struct drm_device *dev = error_priv->dev;
@@ -495,7 +507,7 @@ out:
 	return 0;
 }
 
-int i915_error_state_buf_init(struct drm_i915_error_state_buf *ebuf,
+int i915_error_state_buf_init(struct intel_strbuf *ebuf,
 			      struct drm_i915_private *i915,
 			      size_t count, loff_t pos)
 {
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 4a5af69..68d40ab 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -543,7 +543,7 @@ static ssize_t error_state_read(struct file *filp, struct kobject *kobj,
 	struct drm_minor *minor = dev_to_drm_minor(kdev);
 	struct drm_device *dev = minor->dev;
 	struct i915_error_state_file_priv error_priv;
-	struct drm_i915_error_state_buf error_str;
+	struct intel_strbuf error_str;
 	ssize_t ret_count = 0;
 	int ret;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a9bdc12..2414277 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13762,10 +13762,10 @@ intel_display_capture_error_state(struct drm_device *dev)
 	return error;
 }
 
-#define err_printf(e, ...) i915_error_printf(e, __VA_ARGS__)
+#define err_printf(e, ...) intel_strbuf_printf(e, __VA_ARGS__)
 
 void
-intel_display_print_error_state(struct drm_i915_error_state_buf *m,
+intel_display_print_error_state(struct intel_strbuf *m,
 				struct drm_device *dev,
 				struct intel_display_error_state *error)
 {
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 973c9de..b4fdcc7 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1509,15 +1509,15 @@ err:
 }
 
 void
-intel_overlay_print_error_state(struct drm_i915_error_state_buf *m,
+intel_overlay_print_error_state(struct intel_strbuf *m,
 				struct intel_overlay_error_state *error)
 {
-	i915_error_printf(m, "Overlay, status: 0x%08x, interrupt: 0x%08x\n",
+	intel_strbuf_printf(m, "Overlay, status: 0x%08x, interrupt: 0x%08x\n",
 			  error->dovsta, error->isr);
-	i915_error_printf(m, "  Register file at 0x%08lx:\n",
+	intel_strbuf_printf(m, "  Register file at 0x%08lx:\n",
 			  error->base);
 
-#define P(x) i915_error_printf(m, "    " #x ":	0x%08x\n", error->regs.x)
+#define P(x) intel_strbuf_printf(m, "    " #x ":	0x%08x\n", error->regs.x)
 	P(OBUF_0Y);
 	P(OBUF_1Y);
 	P(OBUF_0U);
-- 
1.9.1

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

Re: [PATCH 3/4] drm/i915: Add i915_gpu_state_unsafe debugfs entry

[Chris Wilson]

On Tue, Dec 09, 2014 at 06:04:33PM +0200, Mika Kuoppala wrote:
> This is a similar to 'i915_gpu_state' but more dangerous
> as it doesn't try to lock nor idle the gpu before grabbing
> the state. But the obvious advantages are that one can
> inspect the current running gpu state and also get a state
> dump even if mutex is hold.
> 
> As we don't want a unsuspecting user to trip into this by
> accident, there needs to be write into the debugfs node
> before any unsafe capturing can take place.

It shouldn't be called unsafe. It is precisely the mode in which the
regular error capture runs and so the kernel is required to be robust.
So just document that i915_gpu_state runs asynchronously to any client
and the gpu, and caveat emptor.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Rename drm_i915_error_state_buf to intel_strbuf

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  364/364              364/364
ILK              +1                 364/366              365/366
SNB                                  448/450              448/450
IVB                                  497/498              497/498
BYT                                  289/289              289/289
HSW                 -1              563/564              562/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 ILK  igt_kms_flip_wf_vblank-ts-check      DMESG_WARN(1, M26)PASS(11, M26M37)      PASS(1, M37)
*HSW  igt_pm_rpm_debugfs-read      PASS(3, M40M20)      DMESG_WARN(1, M20)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915: Add gpu hw state capture helper

[Chris Wilson]

On Tue, Dec 09, 2014 at 06:04:31PM +0200, Mika Kuoppala wrote:
> In the following commits, we want to capture the hw state
> also without any errors. Carve out the helper out from error
> capture parts.

As an aside i915_gpu_state is a better name throughout for struct
drm_i915_error_state. Afterwards, only the i915_gpu_state singleton we
capture upon a hang gets associated with the actual error state.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx