[PATCH 0/9] gpu hang and swizzle patches

[PATCH 0/9] gpu hang and swizzle patches

[Daniel Vetter]

Hi all,

This is a bit a mixed pile, but I've used all the earlier patches to test
the gen6+ swizzling patch and I like to send out patch series somewhat
resembling the setup I've tested them in.

Patches 1-2 refactor our debugfs code a bit.
Patches 3-5 implement a debugfs interface to simulate a gpu hang Patch 6
fixes the swizzle detection on i915G/i945G. This one is for stable and
independent of the previous patches.
Patches 7-8 add a debugfs file with information to debug swizzle issues
Patch 9 implements swizzle support for snb/ivb when in dual-channel mode

Benchmarking on my snb is notoriously difficult, but swizzling seems to
yield a few percent in some workloads, topping out at 5% for padman.

Review highly welcome.

Cheers, Daniel

Daniel Vetter (9):
  drm/i915: refactor debugfs open function
  drm/i915: refactor debugfs create functions
  drm/i915: add interface to simulate gpu hangs
  drm/i915: rework dev->first_error locking
  drm/i915: destroy existing error_state when simulating a gpu hang
  drm/i915: fix swizzle detection for gen3
  drm/i915: add debugfs file for swizzling information
  drm/i915: add gen6+ registers to i915_swizzle_info
  drm/i915: swizzling support for snb/ivb

 drivers/gpu/drm/i915/i915_debugfs.c     |  212 ++++++++++++++++++++++---------
 drivers/gpu/drm/i915/i915_dma.c         |    4 +-
 drivers/gpu/drm/i915/i915_drv.c         |    7 +-
 drivers/gpu/drm/i915/i915_drv.h         |    8 +-
 drivers/gpu/drm/i915/i915_gem.c         |   23 +++-
 drivers/gpu/drm/i915/i915_gem_tiling.c  |   20 +++-
 drivers/gpu/drm/i915/i915_irq.c         |    7 +-
 drivers/gpu/drm/i915/i915_reg.h         |   33 +++++
 drivers/gpu/drm/i915/intel_ringbuffer.c |    4 +
 9 files changed, 249 insertions(+), 69 deletions(-)

-- 
1.7.6.4

[PATCH 1/9] drm/i915: refactor debugfs open function

[Daniel Vetter]

Only forcewake has an open with special semantics, the other r/w
debugfs only assign the file private pointer.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   26 +++++---------------------
 1 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 51b21eb..f37b3ab 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1332,8 +1332,8 @@ static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
 }
 
 static int
-i915_wedged_open(struct inode *inode,
-		 struct file *filp)
+i915_debugfs_common_open(struct inode *inode,
+			 struct file *filp)
 {
 	filp->private_data = inode->i_private;
 	return 0;
@@ -1389,20 +1389,12 @@ i915_wedged_write(struct file *filp,
 
 static const struct file_operations i915_wedged_fops = {
 	.owner = THIS_MODULE,
-	.open = i915_wedged_open,
+	.open = i915_debugfs_common_open,
 	.read = i915_wedged_read,
 	.write = i915_wedged_write,
 	.llseek = default_llseek,
 };
 
-static int
-i915_max_freq_open(struct inode *inode,
-		   struct file *filp)
-{
-	filp->private_data = inode->i_private;
-	return 0;
-}
-
 static ssize_t
 i915_max_freq_read(struct file *filp,
 		   char __user *ubuf,
@@ -1459,20 +1451,12 @@ i915_max_freq_write(struct file *filp,
 
 static const struct file_operations i915_max_freq_fops = {
 	.owner = THIS_MODULE,
-	.open = i915_max_freq_open,
+	.open = i915_debugfs_common_open,
 	.read = i915_max_freq_read,
 	.write = i915_max_freq_write,
 	.llseek = default_llseek,
 };
 
-static int
-i915_cache_sharing_open(struct inode *inode,
-		   struct file *filp)
-{
-	filp->private_data = inode->i_private;
-	return 0;
-}
-
 static ssize_t
 i915_cache_sharing_read(struct file *filp,
 		   char __user *ubuf,
@@ -1538,7 +1522,7 @@ i915_cache_sharing_write(struct file *filp,
 
 static const struct file_operations i915_cache_sharing_fops = {
 	.owner = THIS_MODULE,
-	.open = i915_cache_sharing_open,
+	.open = i915_debugfs_common_open,
 	.read = i915_cache_sharing_read,
 	.write = i915_cache_sharing_write,
 	.llseek = default_llseek,
-- 
1.7.6.4

[PATCH 2/9] drm/i915: refactor debugfs create functions

[Daniel Vetter]

All r/w debugfs files are created equal.

v2: Add some newlines to make the code easier on the eyes as requested
by Ben Widawsky.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   55 +++++++++++-----------------------
 1 files changed, 18 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f37b3ab..648fd64 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1551,21 +1551,6 @@ drm_add_fake_info_node(struct drm_minor *minor,
 	return 0;
 }
 
-static int i915_wedged_create(struct dentry *root, struct drm_minor *minor)
-{
-	struct drm_device *dev = minor->dev;
-	struct dentry *ent;
-
-	ent = debugfs_create_file("i915_wedged",
-				  S_IRUGO | S_IWUSR,
-				  root, dev,
-				  &i915_wedged_fops);
-	if (IS_ERR(ent))
-		return PTR_ERR(ent);
-
-	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;
@@ -1627,34 +1612,22 @@ static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor)
 	return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops);
 }
 
-static int i915_max_freq_create(struct dentry *root, struct drm_minor *minor)
-{
-	struct drm_device *dev = minor->dev;
-	struct dentry *ent;
-
-	ent = debugfs_create_file("i915_max_freq",
-				  S_IRUGO | S_IWUSR,
-				  root, dev,
-				  &i915_max_freq_fops);
-	if (IS_ERR(ent))
-		return PTR_ERR(ent);
-
-	return drm_add_fake_info_node(minor, ent, &i915_max_freq_fops);
-}
-
-static int i915_cache_sharing_create(struct dentry *root, struct drm_minor *minor)
+static int i915_debugfs_create(struct dentry *root,
+			       struct drm_minor *minor,
+			       const char *name,
+			       const struct file_operations *fops)
 {
 	struct drm_device *dev = minor->dev;
 	struct dentry *ent;
 
-	ent = debugfs_create_file("i915_cache_sharing",
+	ent = debugfs_create_file(name,
 				  S_IRUGO | S_IWUSR,
 				  root, dev,
-				  &i915_cache_sharing_fops);
+				  fops);
 	if (IS_ERR(ent))
 		return PTR_ERR(ent);
 
-	return drm_add_fake_info_node(minor, ent, &i915_cache_sharing_fops);
+	return drm_add_fake_info_node(minor, ent, fops);
 }
 
 static struct drm_info_list i915_debugfs_list[] = {
@@ -1703,17 +1676,25 @@ int i915_debugfs_init(struct drm_minor *minor)
 {
 	int ret;
 
-	ret = i915_wedged_create(minor->debugfs_root, minor);
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_wedged",
+				  &i915_wedged_fops);
 	if (ret)
 		return ret;
 
 	ret = i915_forcewake_create(minor->debugfs_root, minor);
 	if (ret)
 		return ret;
-	ret = i915_max_freq_create(minor->debugfs_root, minor);
+
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_max_freq",
+				  &i915_max_freq_fops);
 	if (ret)
 		return ret;
-	ret = i915_cache_sharing_create(minor->debugfs_root, minor);
+
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_cache_sharing",
+				  &i915_cache_sharing_fops);
 	if (ret)
 		return ret;
 
-- 
1.7.6.4

[PATCH 3/9] drm/i915: add interface to simulate gpu hangs

[Daniel Vetter]

gpu reset is a very important piece of our infrastructure.
Unfortunately we only really it test by actually hanging the gpu,
which often has bad side-effects for the entire system. And the gpu
hang handling code is one of the rather complicated pieces of code we
have, consisting of
- hang detection
- error capture
- actual gpu reset
- reset of all the gem bookkeeping
- reinitialition of the entire gpu

This patch adds a debugfs to selectively stopping rings by ceasing to
update the hw tail pointer, which will result in the gpu no longer
updating it's head pointer and eventually to the hangcheck firing.
This way we can exercise the gpu hang code under controlled conditions
without a dying gpu taking down the entire systems.

Patch motivated by me forgetting to properly reinitialize ppgtt after
a gpu reset.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |   63 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.c         |    3 +
 drivers/gpu/drm/i915/i915_drv.h         |    2 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |    4 ++
 4 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 648fd64..e1c5aa1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1396,6 +1396,64 @@ static const struct file_operations i915_wedged_fops = {
 };
 
 static ssize_t
+i915_ring_stop_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),
+		       "%d\n", dev_priv->stop_rings);
+
+	if (len > sizeof(buf))
+		len = sizeof(buf);
+
+	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+}
+
+static ssize_t
+i915_ring_stop_write(struct file *filp,
+		     const char __user *ubuf,
+		     size_t cnt,
+		     loff_t *ppos)
+{
+	struct drm_device *dev = filp->private_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	char buf[20];
+	int val = 0;
+
+	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("Stopping rings %u\n", val);
+
+	mutex_lock(&dev->struct_mutex);
+	dev_priv->stop_rings = val;
+	mutex_unlock(&dev->struct_mutex);
+
+	return cnt;
+}
+
+static const struct file_operations i915_ring_stop_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_debugfs_common_open,
+	.read = i915_ring_stop_read,
+	.write = i915_ring_stop_write,
+	.llseek = default_llseek,
+};
+static ssize_t
 i915_max_freq_read(struct file *filp,
 		   char __user *ubuf,
 		   size_t max,
@@ -1697,6 +1755,11 @@ int i915_debugfs_init(struct drm_minor *minor)
 				  &i915_cache_sharing_fops);
 	if (ret)
 		return ret;
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_ring_stop",
+				  &i915_ring_stop_fops);
+	if (ret)
+		return ret;
 
 	return drm_debugfs_create_files(i915_debugfs_list,
 					I915_DEBUGFS_ENTRIES,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index bab4e08..e8e4cd0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -616,6 +616,9 @@ int i915_reset(struct drm_device *dev, u8 flags)
 	if (!mutex_trylock(&dev->struct_mutex))
 		return -EBUSY;
 
+	printk("reenabling rings\n");
+	dev_priv->stop_rings = 0;
+
 	i915_gem_reset(dev);
 
 	ret = -ENODEV;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 25036f5..8bb83c0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -336,6 +336,8 @@ typedef struct drm_i915_private {
 	uint32_t last_instdone;
 	uint32_t last_instdone1;
 
+	unsigned int stop_rings;
+
 	unsigned long cfb_size;
 	unsigned int cfb_fb;
 	enum plane cfb_plane;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2d476a9..cf6e159 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1179,7 +1179,11 @@ int intel_ring_begin(struct intel_ring_buffer *ring,
 
 void intel_ring_advance(struct intel_ring_buffer *ring)
 {
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
 	ring->tail &= ring->size - 1;
+	if (dev_priv->stop_rings & intel_ring_flag(ring))
+		return;
 	ring->write_tail(ring, ring->tail);
 }
 
-- 
1.7.6.4

[PATCH 4/9] drm/i915: rework dev->first_error locking

[Daniel Vetter]

- reduce the irq disabled section, even for a debugfs file this was
  way too long.
- protect readers of the captured error state from concurrent freeing
  of the same by holding dev->struct_mutex.
- always disable irqs when taking the lock.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |    9 ++++++---
 drivers/gpu/drm/i915/i915_dma.c     |    2 ++
 drivers/gpu/drm/i915/i915_drv.h     |    3 +++
 drivers/gpu/drm/i915/i915_irq.c     |    7 +++++--
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e1c5aa1..63e1c36 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -768,13 +768,16 @@ static int i915_error_state(struct seq_file *m, void *unused)
 	unsigned long flags;
 	int i, page, offset, elt;
 
+	mutex_lock(&dev->struct_mutex);
 	spin_lock_irqsave(&dev_priv->error_lock, flags);
-	if (!dev_priv->first_error) {
+	error = dev_priv->first_error;
+	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+
+	if (!error) {
 		seq_printf(m, "no error state collected\n");
 		goto out;
 	}
 
-	error = dev_priv->first_error;
 
 	seq_printf(m, "Time: %ld s %ld us\n", error->time.tv_sec,
 		   error->time.tv_usec);
@@ -846,7 +849,7 @@ static int i915_error_state(struct seq_file *m, void *unused)
 		intel_display_print_error_state(m, dev, error->display);
 
 out:
-	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+	mutex_unlock(&dev->struct_mutex);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index ab3a3fd..0034c6a 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2153,7 +2153,9 @@ int i915_driver_unload(struct drm_device *dev)
 	/* Free error state after interrupts are fully disabled. */
 	del_timer_sync(&dev_priv->hangcheck_timer);
 	cancel_work_sync(&dev_priv->error_work);
+	mutex_lock(&dev->struct_mutex);
 	i915_destroy_error_state(dev);
+	mutex_unlock(&dev->struct_mutex);
 
 	if (dev->pdev->msi_enabled)
 		pci_disable_msi(dev->pdev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8bb83c0..660b62c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -386,6 +386,9 @@ typedef struct drm_i915_private {
 	unsigned int fsb_freq, mem_freq, is_ddr3;
 
 	spinlock_t error_lock;
+	/* Protected by dev->error_lock. To ensure that the error_state obtained
+	 * through this pointer doesn't disappear, you also need to hold
+	 * dev->struct_mutex */
 	struct drm_i915_error_state *first_error;
 	struct work_struct error_work;
 	struct completion error_completion;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a04d606..c9b0766 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1024,11 +1024,14 @@ void i915_destroy_error_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_error_state *error;
+	unsigned long flags;
+
+	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
 
-	spin_lock(&dev_priv->error_lock);
+	spin_lock_irqsave(&dev_priv->error_lock, flags);
 	error = dev_priv->first_error;
 	dev_priv->first_error = NULL;
-	spin_unlock(&dev_priv->error_lock);
+	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
 
 	if (error)
 		i915_error_state_free(dev, error);
-- 
1.7.6.4

[PATCH 5/9] drm/i915: destroy existing error_state when simulating a gpu hang

[Daniel Vetter]

This way we can simulate a bunch of gpu hangs and run the error_state
capture code every time (without the need to reload the module).

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 63e1c36..2ad2237 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1443,6 +1443,8 @@ i915_ring_stop_write(struct file *filp,
 	DRM_DEBUG_DRIVER("Stopping rings %u\n", val);
 
 	mutex_lock(&dev->struct_mutex);
+	i915_destroy_error_state(dev);
+
 	dev_priv->stop_rings = val;
 	mutex_unlock(&dev->struct_mutex);
 
-- 
1.7.6.4

[PATCH 6/9] drm/i915: fix swizzle detection for gen3

[Daniel Vetter]

It looks like the desktop variants of i915 and i945 also have the DCC
register to control dram channel interleave and cpu side bit6
swizzling.

Unfurnately internal Cspec/ConfigDB documentation for these ancient chips
have already been dropped and there seem to be no archives. Also
somebody thought the swizzling behaviour is surely a worthy secret to
keep and redacted any mention of these fields from the published Intel
datasheets.

I suspect the hw engineers were really proud of the page coloring
they've achieved in their first dual channel dram controller with
bit17 - after all Bspec explains in great length the optimal layout of
page frame numbers modulo 4 for the color and depth buffers, too.
Later on when they've started to work on VT-d they shamefully
discoverd their stupidity and tried to cover the tracks ...

Tested-by: Daniel Vetter <daniel.vetter@ffwll.ch> (i915g)
Tested-by: Pavel Ondračka <pavel.ondracka@email.cz> (i945g)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42625
Cc: stable@kernel.org
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_tiling.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 31d334d..861223b 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -107,10 +107,10 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
 		 */
 		swizzle_x = I915_BIT_6_SWIZZLE_NONE;
 		swizzle_y = I915_BIT_6_SWIZZLE_NONE;
-	} else if (IS_MOBILE(dev)) {
+	} else if (IS_MOBILE(dev) || (IS_GEN3(dev) && !IS_G33(dev))) {
 		uint32_t dcc;
 
-		/* On mobile 9xx chipsets, channel interleave by the CPU is
+		/* On 9xx chipsets, channel interleave by the CPU is
 		 * determined by DCC.  For single-channel, neither the CPU
 		 * nor the GPU do swizzling.  For dual channel interleaved,
 		 * the GPU's interleave is bit 9 and 10 for X tiled, and bit
-- 
1.7.6.4

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

[PATCH 7/9] drm/i915: add debugfs file for swizzling information

[Daniel Vetter]

This will also come handy for the gen6+ swizzling support, where the
driver is supposed to control swizzling depending upon dram
configuration.

v2: CxDRB3 are 16 bit regs! Noticed by Chris Wilson.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   50 +++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2ad2237..d5bc92b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1334,6 +1334,55 @@ static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
 	return 0;
 }
 
+static const char *swizzle_string(unsigned swizzle)
+{
+	switch(swizzle) {
+		case I915_BIT_6_SWIZZLE_NONE:
+			return "none";
+		case I915_BIT_6_SWIZZLE_9:
+			return "bit9";
+		case I915_BIT_6_SWIZZLE_9_10:
+			return "bit9/bit10";
+		case I915_BIT_6_SWIZZLE_9_11:
+			return "bit9/bit11";
+		case I915_BIT_6_SWIZZLE_9_10_11:
+			return "bit9/bit10/bit11";
+		case I915_BIT_6_SWIZZLE_9_17:
+			return "bit9/bit17";
+		case I915_BIT_6_SWIZZLE_9_10_17:
+			return "bit9/bit10/bit17";
+		case I915_BIT_6_SWIZZLE_UNKNOWN:
+			return "unkown";
+	}
+
+	return "bug";
+}
+
+static int i915_swizzle_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;
+
+	mutex_lock(&dev->struct_mutex);
+	seq_printf(m, "bit6 swizzle for X-tiling = %s\n",
+		   swizzle_string(dev_priv->mm.bit_6_swizzle_x));
+	seq_printf(m, "bit6 swizzle for Y-tiling = %s\n",
+		   swizzle_string(dev_priv->mm.bit_6_swizzle_y));
+
+	if (IS_GEN3(dev) || IS_GEN4(dev)) {
+		seq_printf(m, "DDC = 0x%08x\n",
+			   I915_READ(DCC));
+		seq_printf(m, "C0DRB3 = 0x%04x\n",
+			   I915_READ16(C0DRB3));
+		seq_printf(m, "C1DRB3 = 0x%04x\n",
+			   I915_READ16(C1DRB3));
+	}
+	mutex_unlock(&dev->struct_mutex);
+
+	return 0;
+}
+
 static int
 i915_debugfs_common_open(struct inode *inode,
 			 struct file *filp)
@@ -1732,6 +1781,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_gem_framebuffer", i915_gem_framebuffer_info, 0},
 	{"i915_context_status", i915_context_status, 0},
 	{"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0},
+	{"i915_swizzle_info", i915_swizzle_info, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
-- 
1.7.6.4

[PATCH 8/9] drm/i915: add gen6+ registers to i915_swizzle_info

[Daniel Vetter]

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   13 +++++++++++++
 drivers/gpu/drm/i915/i915_reg.h     |   31 +++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d5bc92b..41b455b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1377,6 +1377,19 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
 			   I915_READ16(C0DRB3));
 		seq_printf(m, "C1DRB3 = 0x%04x\n",
 			   I915_READ16(C1DRB3));
+	} else if (IS_GEN6(dev) || IS_GEN7(dev)) {
+		seq_printf(m, "MAD_DIMM_C0 = 0x%08x\n",
+			   I915_READ(MAD_DIMM_C0));
+		seq_printf(m, "MAD_DIMM_C1 = 0x%08x\n",
+			   I915_READ(MAD_DIMM_C1));
+		seq_printf(m, "MAD_DIMM_C2 = 0x%08x\n",
+			   I915_READ(MAD_DIMM_C2));
+		seq_printf(m, "TILECTL = 0x%08x\n",
+			   I915_READ(TILECTL));
+		seq_printf(m, "ARB_MODE = 0x%08x\n",
+			   I915_READ(ARB_MODE));
+		seq_printf(m, "DISP_ARB_CTL = 0x%08x\n",
+			   I915_READ(DISP_ARB_CTL));
 	}
 	mutex_unlock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cbf5f9f..0a0b6b1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -295,6 +295,12 @@
 #define FENCE_REG_SANDYBRIDGE_0		0x100000
 #define   SANDYBRIDGE_FENCE_PITCH_SHIFT	32
 
+/* control register for cpu gtt access */
+#define TILECTL				0x101000
+#define   TILECTL_SWZCTL			(1 << 0)
+#define   TILECTL_TLB_PREFETCH_DIS	(1 << 2)
+#define   TILECTL_BACKSNOOP_DIS		(1 << 3)
+
 /*
  * Instruction and interrupt control regs
  */
@@ -318,6 +324,9 @@
 #define RING_MAX_IDLE(base)	((base)+0x54)
 #define RING_HWS_PGA(base)	((base)+0x80)
 #define RING_HWS_PGA_GEN6(base)	((base)+0x2080)
+#define ARB_MODE		0x04030
+#define   ARB_MODE_SWIZZLE_SNB	(1<<4)
+#define   ARB_MODE_SWIZZLE_IVB	(1<<5)
 #define RENDER_HWS_PGA_GEN7	(0x04080)
 #define BSD_HWS_PGA_GEN7	(0x04180)
 #define BLT_HWS_PGA_GEN7	(0x04280)
@@ -1034,6 +1043,28 @@
 #define C0DRB3			0x10206
 #define C1DRB3			0x10606
 
+/** snb MCH registers for reading the DRAM channel configuration */
+#define MAD_DIMM_C0			(MCHBAR_MIRROR_BASE_SNB + 0x5004)
+#define   MAD_DIMM_C1			(MCHBAR_MIRROR_BASE_SNB + 0x5008)
+#define   MAD_DIMM_C2			(MCHBAR_MIRROR_BASE_SNB + 0x500C)
+#define   MAD_DIMM_ECC_MASK		(0x3 << 24)
+#define   MAD_DIMM_ECC_OFF		(0x0 << 24)
+#define   MAD_DIMM_ECC_IO_ON_LOGIC_OFF	(0x1 << 24)
+#define   MAD_DIMM_ECC_IO_OFF_LOGIC_ON	(0x2 << 24)
+#define   MAD_DIMM_ECC_ON		(0x3 << 24)
+#define   MAD_DIMM_ENH_INTERLEAVE	(0x1 << 22)
+#define   MAD_DIMM_RANK_INTERLEAVE	(0x1 << 21)
+#define   MAD_DIMM_B_WIDTH_X16		(0x1 << 20) /* X8 chips if unset */
+#define   MAD_DIMM_A_WIDTH_X16		(0x1 << 19) /* X8 chips if unset */
+#define   MAD_DIMM_B_DUAL_RANK		(0x1 << 18)
+#define   MAD_DIMM_A_DUAL_RANK		(0x1 << 17)
+#define   MAD_DIMM_A_SELECT		(0x1 << 16)
+#define   MAD_DIMM_B_SIZE_MASK		(0xff << 8) /* in multiples of 256mb */
+#define   MAD_DIMM_B_SIZE_SHIFT		8
+#define   MAD_DIMM_A_SIZE_MASK		(0xff << 0) /* in multiples of 256mb */
+#define   MAD_DIMM_A_SIZE_SHIFT		8
+
+
 /* Clocking configuration register */
 #define CLKCFG			0x10c00
 #define CLKCFG_FSB_400					(5 << 0)	/* hrawclk 100 */
-- 
1.7.6.4

[PATCH 9/9] drm/i915: swizzling support for snb/ivb

[Daniel Vetter]

We have to do this manually. Somebody had a Great Idea.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_dma.c        |    2 +-
 drivers/gpu/drm/i915/i915_drv.c        |    4 +++-
 drivers/gpu/drm/i915/i915_drv.h        |    3 ++-
 drivers/gpu/drm/i915/i915_gem.c        |   23 +++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_gem_tiling.c |   16 ++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h        |    2 ++
 6 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0034c6a..a2a107d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1201,7 +1201,7 @@ static int i915_load_gem_init(struct drm_device *dev)
 	i915_gem_do_init(dev, 0, mappable_size, gtt_size - PAGE_SIZE);
 
 	mutex_lock(&dev->struct_mutex);
-	ret = i915_gem_init_ringbuffer(dev);
+	ret = i915_gem_init_hw(dev);
 	mutex_unlock(&dev->struct_mutex);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e8e4cd0..8ce04b7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -472,7 +472,7 @@ static int i915_drm_thaw(struct drm_device *dev)
 		mutex_lock(&dev->struct_mutex);
 		dev_priv->mm.suspended = 0;
 
-		error = i915_gem_init_ringbuffer(dev);
+		error = i915_gem_init_hw(dev);
 		mutex_unlock(&dev->struct_mutex);
 
 		if (HAS_PCH_SPLIT(dev))
@@ -669,6 +669,8 @@ int i915_reset(struct drm_device *dev, u8 flags)
 			!dev_priv->mm.suspended) {
 		dev_priv->mm.suspended = 0;
 
+		i915_gem_init_swizzling(dev);
+
 		dev_priv->ring[RCS].init(&dev_priv->ring[RCS]);
 		if (HAS_BSD(dev))
 		    dev_priv->ring[VCS].init(&dev_priv->ring[VCS]);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 660b62c..9170463 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1177,7 +1177,8 @@ int __must_check i915_gem_object_set_domain(struct drm_i915_gem_object *obj,
 					    uint32_t read_domains,
 					    uint32_t write_domain);
 int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
-int __must_check i915_gem_init_ringbuffer(struct drm_device *dev);
+int __must_check i915_gem_init_hw(struct drm_device *dev);
+void i915_gem_init_swizzling(struct drm_device *dev);
 void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
 void i915_gem_do_init(struct drm_device *dev,
 		      unsigned long start,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a838597..c9d9b62 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3739,12 +3739,31 @@ i915_gem_idle(struct drm_device *dev)
 	return 0;
 }
 
+void i915_gem_init_swizzling(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+
+	if (INTEL_INFO(dev)->gen < 6 ||
+	    dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_NONE)
+		return;
+
+	I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_SWZCTL);
+	if (IS_GEN6(dev))
+		I915_WRITE(ARB_MODE, ARB_MODE_ENABLE(ARB_MODE_SWIZZLE_SNB));
+	else
+		I915_WRITE(ARB_MODE, ARB_MODE_ENABLE(ARB_MODE_SWIZZLE_IVB));
+	I915_WRITE(DISP_ARB_CTL, I915_READ(DISP_ARB_CTL) |
+				 DISP_TILE_SURFACE_SWIZZLING);
+
+}
 int
-i915_gem_init_ringbuffer(struct drm_device *dev)
+i915_gem_init_hw(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	int ret;
 
+	i915_gem_init_swizzling(dev);
+
 	ret = intel_init_render_ring_buffer(dev);
 	if (ret)
 		return ret;
@@ -3800,7 +3819,7 @@ i915_gem_entervt_ioctl(struct drm_device *dev, void *data,
 	mutex_lock(&dev->struct_mutex);
 	dev_priv->mm.suspended = 0;
 
-	ret = i915_gem_init_ringbuffer(dev);
+	ret = i915_gem_init_hw(dev);
 	if (ret != 0) {
 		mutex_unlock(&dev->struct_mutex);
 		return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 861223b..af0a2fc 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -93,8 +93,20 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
 	uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
 
 	if (INTEL_INFO(dev)->gen >= 6) {
-		swizzle_x = I915_BIT_6_SWIZZLE_NONE;
-		swizzle_y = I915_BIT_6_SWIZZLE_NONE;
+		uint32_t dimm_c0, dimm_c1;
+		dimm_c0 = I915_READ(MAD_DIMM_C0);
+		dimm_c1 = I915_READ(MAD_DIMM_C1);
+		dimm_c0 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_A_SIZE_MASK;
+		dimm_c1 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_A_SIZE_MASK;
+		/* Enable swizzling when the channels are populated with
+		 * identically sized dimms. */
+		if (dimm_c0 == dimm_c1) {
+			swizzle_x = I915_BIT_6_SWIZZLE_9_10;
+			swizzle_y = I915_BIT_6_SWIZZLE_9;
+		} else {
+			swizzle_x = I915_BIT_6_SWIZZLE_NONE;
+			swizzle_y = I915_BIT_6_SWIZZLE_NONE;
+		}
 	} else if (IS_GEN5(dev)) {
 		/* On Ironlake whatever DRAM config, GPU always do
 		 * same swizzling setup.
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0a0b6b1..a62fa95 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -327,6 +327,8 @@
 #define ARB_MODE		0x04030
 #define   ARB_MODE_SWIZZLE_SNB	(1<<4)
 #define   ARB_MODE_SWIZZLE_IVB	(1<<5)
+#define   ARB_MODE_ENABLE(x)	GFX_MODE_ENABLE(x)
+#define   ARB_MODE_DISABLE(x)	GFX_MODE_DISABLE(x)
 #define RENDER_HWS_PGA_GEN7	(0x04080)
 #define BSD_HWS_PGA_GEN7	(0x04180)
 #define BLT_HWS_PGA_GEN7	(0x04280)
-- 
1.7.6.4

[PATCH] drm/i915: add interface to simulate gpu hangs

[Daniel Vetter]

gpu reset is a very important piece of our infrastructure.
Unfortunately we only really it test by actually hanging the gpu,
which often has bad side-effects for the entire system. And the gpu
hang handling code is one of the rather complicated pieces of code we
have, consisting of
- hang detection
- error capture
- actual gpu reset
- reset of all the gem bookkeeping
- reinitialition of the entire gpu

This patch adds a debugfs to selectively stopping rings by ceasing to
update the hw tail pointer, which will result in the gpu no longer
updating it's head pointer and eventually to the hangcheck firing.
This way we can exercise the gpu hang code under controlled conditions
without a dying gpu taking down the entire systems.

Patch motivated by me forgetting to properly reinitialize ppgtt after
a gpu reset.

Usage:

echo $((1 << $ringnum)) > i915_ring_stop # stops one ring

echo 0xffffffff > i915_ring_stop # stops all, future-proof version

then run whatever testload is desired. i915_ring_stop automatically
resets after a gpu hang is detected to avoid hanging the gpu to fast
and declaring it wedged.

v2: Incorporate feedback from Chris Wilson.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |   63 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.c         |    2 +
 drivers/gpu/drm/i915/i915_drv.h         |    2 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |    4 ++
 4 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 648fd64..e1c5aa1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1396,6 +1396,64 @@ static const struct file_operations i915_wedged_fops = {
 };
 
 static ssize_t
+i915_ring_stop_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),
+		       "%d\n", dev_priv->stop_rings);
+
+	if (len > sizeof(buf))
+		len = sizeof(buf);
+
+	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+}
+
+static ssize_t
+i915_ring_stop_write(struct file *filp,
+		     const char __user *ubuf,
+		     size_t cnt,
+		     loff_t *ppos)
+{
+	struct drm_device *dev = filp->private_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	char buf[20];
+	int val = 0;
+
+	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("Stopping rings %u\n", val);
+
+	mutex_lock(&dev->struct_mutex);
+	dev_priv->stop_rings = val;
+	mutex_unlock(&dev->struct_mutex);
+
+	return cnt;
+}
+
+static const struct file_operations i915_ring_stop_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_debugfs_common_open,
+	.read = i915_ring_stop_read,
+	.write = i915_ring_stop_write,
+	.llseek = default_llseek,
+};
+static ssize_t
 i915_max_freq_read(struct file *filp,
 		   char __user *ubuf,
 		   size_t max,
@@ -1697,6 +1755,11 @@ int i915_debugfs_init(struct drm_minor *minor)
 				  &i915_cache_sharing_fops);
 	if (ret)
 		return ret;
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_ring_stop",
+				  &i915_ring_stop_fops);
+	if (ret)
+		return ret;
 
 	return drm_debugfs_create_files(i915_debugfs_list,
 					I915_DEBUGFS_ENTRIES,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index bab4e08..ac2d5e8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -616,6 +616,8 @@ int i915_reset(struct drm_device *dev, u8 flags)
 	if (!mutex_trylock(&dev->struct_mutex))
 		return -EBUSY;
 
+	dev_priv->stop_rings = 0;
+
 	i915_gem_reset(dev);
 
 	ret = -ENODEV;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 25036f5..8bb83c0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -336,6 +336,8 @@ typedef struct drm_i915_private {
 	uint32_t last_instdone;
 	uint32_t last_instdone1;
 
+	unsigned int stop_rings;
+
 	unsigned long cfb_size;
 	unsigned int cfb_fb;
 	enum plane cfb_plane;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2d476a9..cf6e159 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1179,7 +1179,11 @@ int intel_ring_begin(struct intel_ring_buffer *ring,
 
 void intel_ring_advance(struct intel_ring_buffer *ring)
 {
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
 	ring->tail &= ring->size - 1;
+	if (dev_priv->stop_rings & intel_ring_flag(ring))
+		return;
 	ring->write_tail(ring, ring->tail);
 }
 
-- 
1.7.6.4

[PATCH] drm/i915: fix swizzle detection for gen3

[Daniel Vetter]

It looks like the desktop variants of i915 and i945 also have the DCC
register to control dram channel interleave and cpu side bit6
swizzling.

Unfortunately internal Cspec/ConfigDB documentation for these ancient chips
have already been dropped and there seem to be no archives. Also
somebody thought the swizzling behaviour is surely a worthy secret to
keep and redacted any mention of these fields from the published Intel
datasheets.

I suspect the hw engineers were really proud of the page coloring
they've achieved in their first dual channel dram controller with
bit17 - after all Bspec explains in great length the optimal layout of
page frame numbers modulo 4 for the color and depth buffers, too.
Later on when they've started to work on VT-d they shamefully
discoverd their stupidity and tried to cover the tracks ...

Tested-by: Daniel Vetter <daniel.vetter@ffwll.ch> (i915g)
Tested-by: Pavel Ondračka <pavel.ondracka@email.cz> (i945g)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42625
Cc: stable@kernel.org
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---

v2: Spelling fix in the commit msg.

 drivers/gpu/drm/i915/i915_gem_tiling.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 31d334d..861223b 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -107,10 +107,10 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
 		 */
 		swizzle_x = I915_BIT_6_SWIZZLE_NONE;
 		swizzle_y = I915_BIT_6_SWIZZLE_NONE;
-	} else if (IS_MOBILE(dev)) {
+	} else if (IS_MOBILE(dev) || (IS_GEN3(dev) && !IS_G33(dev))) {
 		uint32_t dcc;
 
-		/* On mobile 9xx chipsets, channel interleave by the CPU is
+		/* On 9xx chipsets, channel interleave by the CPU is
 		 * determined by DCC.  For single-channel, neither the CPU
 		 * nor the GPU do swizzling.  For dual channel interleaved,
 		 * the GPU's interleave is bit 9 and 10 for X tiled, and bit
-- 
1.7.6.4

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

[PATCH] drm/i915: add debugfs file for swizzling information

[Daniel Vetter]

This will also come handy for the gen6+ swizzling support, where the
driver is supposed to control swizzling depending upon dram
configuration.

v2: CxDRB3 are 16 bit regs! Noticed by Chris Wilson.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---

v3: align case blocks with the switch statement.

 drivers/gpu/drm/i915/i915_debugfs.c |   50 +++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2ad2237..a0659f9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1334,6 +1334,55 @@ static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
 	return 0;
 }
 
+static const char *swizzle_string(unsigned swizzle)
+{
+	switch(swizzle) {
+	case I915_BIT_6_SWIZZLE_NONE:
+		return "none";
+	case I915_BIT_6_SWIZZLE_9:
+		return "bit9";
+	case I915_BIT_6_SWIZZLE_9_10:
+		return "bit9/bit10";
+	case I915_BIT_6_SWIZZLE_9_11:
+		return "bit9/bit11";
+	case I915_BIT_6_SWIZZLE_9_10_11:
+		return "bit9/bit10/bit11";
+	case I915_BIT_6_SWIZZLE_9_17:
+		return "bit9/bit17";
+	case I915_BIT_6_SWIZZLE_9_10_17:
+		return "bit9/bit10/bit17";
+	case I915_BIT_6_SWIZZLE_UNKNOWN:
+		return "unkown";
+	}
+
+	return "bug";
+}
+
+static int i915_swizzle_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;
+
+	mutex_lock(&dev->struct_mutex);
+	seq_printf(m, "bit6 swizzle for X-tiling = %s\n",
+		   swizzle_string(dev_priv->mm.bit_6_swizzle_x));
+	seq_printf(m, "bit6 swizzle for Y-tiling = %s\n",
+		   swizzle_string(dev_priv->mm.bit_6_swizzle_y));
+
+	if (IS_GEN3(dev) || IS_GEN4(dev)) {
+		seq_printf(m, "DDC = 0x%08x\n",
+			   I915_READ(DCC));
+		seq_printf(m, "C0DRB3 = 0x%04x\n",
+			   I915_READ16(C0DRB3));
+		seq_printf(m, "C1DRB3 = 0x%04x\n",
+			   I915_READ16(C1DRB3));
+	}
+	mutex_unlock(&dev->struct_mutex);
+
+	return 0;
+}
+
 static int
 i915_debugfs_common_open(struct inode *inode,
 			 struct file *filp)
@@ -1732,6 +1781,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_gem_framebuffer", i915_gem_framebuffer_info, 0},
 	{"i915_context_status", i915_context_status, 0},
 	{"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0},
+	{"i915_swizzle_info", i915_swizzle_info, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
-- 
1.7.6.4

Re: [PATCH 1/9] drm/i915: refactor debugfs open function

[Ben Widawsky]

On Thu, Nov 10, 2011 at 02:17:59PM +0100, Daniel Vetter wrote:
> Only forcewake has an open with special semantics, the other r/w
> debugfs only assign the file private pointer.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Re: [PATCH 2/9] drm/i915: refactor debugfs create functions

[Ben Widawsky]

On Thu, Nov 10, 2011 at 02:18:00PM +0100, Daniel Vetter wrote:
> All r/w debugfs files are created equal.
> 
> v2: Add some newlines to make the code easier on the eyes as requested
> by Ben Widawsky.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Re: [PATCH 0/9] gpu hang and swizzle patches

[Chris Wilson]

On Thu, 10 Nov 2011 14:17:58 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
> 
> This is a bit a mixed pile, but I've used all the earlier patches to test
> the gen6+ swizzling patch and I like to send out patch series somewhat
> resembling the setup I've tested them in.
> 
> Patches 1-2 refactor our debugfs code a bit.
> Patches 3-5 implement a debugfs interface to simulate a gpu hang Patch 6
> fixes the swizzle detection on i915G/i945G. This one is for stable and
> independent of the previous patches.
> Patches 7-8 add a debugfs file with information to debug swizzle issues
> Patch 9 implements swizzle support for snb/ivb when in dual-channel mode
> 
> Benchmarking on my snb is notoriously difficult, but swizzling seems to
> yield a few percent in some workloads, topping out at 5% for padman.
> 
> Review highly welcome.

I've had a play and could barely detect any improvement above the noise.
However, I've read through all the patches and passed on my comments and
I don't see anything fundamentally wrong with them (though I see enough
GPU hangs not to need to manually trigger one ;-).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 9/9] drm/i915: swizzling support for snb/ivb

[Eric Anholt]

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

On Thu, 10 Nov 2011 14:18:07 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We have to do this manually. Somebody had a Great Idea.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

People playing with this when not strictly required is scary to me.
Manually swizzling was a world of hurt.  I got to play with things like
"when the management engine is enabled, it carves out the top N MB of
one of the dimms, and the corresponding N MB of the other dimm doesn't
get swizzled, and you lose".

> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index 861223b..af0a2fc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -93,8 +93,20 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
>  	uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
>  
>  	if (INTEL_INFO(dev)->gen >= 6) {
> -		swizzle_x = I915_BIT_6_SWIZZLE_NONE;
> -		swizzle_y = I915_BIT_6_SWIZZLE_NONE;
> +		uint32_t dimm_c0, dimm_c1;
> +		dimm_c0 = I915_READ(MAD_DIMM_C0);
> +		dimm_c1 = I915_READ(MAD_DIMM_C1);
> +		dimm_c0 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_A_SIZE_MASK;
> +		dimm_c1 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_A_SIZE_MASK;
> +		/* Enable swizzling when the channels are populated with
> +		 * identically sized dimms. */
> +		if (dimm_c0 == dimm_c1) {
> +			swizzle_x = I915_BIT_6_SWIZZLE_9_10;
> +			swizzle_y = I915_BIT_6_SWIZZLE_9;
> +		} else {
> +			swizzle_x = I915_BIT_6_SWIZZLE_NONE;
> +			swizzle_y = I915_BIT_6_SWIZZLE_NONE;
> +		}
>  	} else if (IS_GEN5(dev)) {
>  		/* On Ironlake whatever DRAM config, GPU always do
>  		 * same swizzling setup.
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0a0b6b1..a62fa95 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -327,6 +327,8 @@
>  #define ARB_MODE		0x04030
>  #define   ARB_MODE_SWIZZLE_SNB	(1<<4)
>  #define   ARB_MODE_SWIZZLE_IVB	(1<<5)
> +#define   ARB_MODE_ENABLE(x)	GFX_MODE_ENABLE(x)
> +#define   ARB_MODE_DISABLE(x)	GFX_MODE_DISABLE(x)
>  #define RENDER_HWS_PGA_GEN7	(0x04080)
>  #define BSD_HWS_PGA_GEN7	(0x04180)
>  #define BLT_HWS_PGA_GEN7	(0x04280)

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 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

Re: [PATCH 9/9] drm/i915: swizzling support for snb/ivb

[Daniel Vetter]

On Fri, Nov 11, 2011 at 08:50:30AM -0800, Eric Anholt wrote:
> On Thu, 10 Nov 2011 14:18:07 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > We have to do this manually. Somebody had a Great Idea.
> > 
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> People playing with this when not strictly required is scary to me.
> Manually swizzling was a world of hurt.  I got to play with things like
> "when the management engine is enabled, it carves out the top N MB of
> one of the dimms, and the corresponding N MB of the other dimm doesn't
> get swizzled, and you lose".

Looks like yet another patch series of mine that scares away people ...

Would this patch be less scary when we have a test that slurps in the
entire ram to quickly diagnose such issues? We can then either revert this
or fix up the detection to not enable swizzling in such cases.

Also the manually swizzling is a world of hurt argument is pretty void: Up
to very recent kernels we've advertised bit9 swizzling on snb+ without any
swizzling actually going on. So userspace clearly doesn't rely on this
anymore (the issue was caught by running the pread tests in i-g-t).

Also we already have a bug for gm45 which looks like a portion of ram
isn't swizzled:
https://bugs.freedesktop.org/show_bug.cgi?id=28813

-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 9/9] drm/i915: swizzling support for snb/ivb

[Eric Anholt]

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

On Fri, 11 Nov 2011 18:22:25 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Nov 11, 2011 at 08:50:30AM -0800, Eric Anholt wrote:
> > On Thu, 10 Nov 2011 14:18:07 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > We have to do this manually. Somebody had a Great Idea.
> > > 
> > > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > People playing with this when not strictly required is scary to me.
> > Manually swizzling was a world of hurt.  I got to play with things like
> > "when the management engine is enabled, it carves out the top N MB of
> > one of the dimms, and the corresponding N MB of the other dimm doesn't
> > get swizzled, and you lose".
> 
> Looks like yet another patch series of mine that scares away people ...
> 
> Would this patch be less scary when we have a test that slurps in the
> entire ram to quickly diagnose such issues? We can then either revert this
> or fix up the detection to not enable swizzling in such cases.
> 
> Also the manually swizzling is a world of hurt argument is pretty void: Up
> to very recent kernels we've advertised bit9 swizzling on snb+ without any
> swizzling actually going on. So userspace clearly doesn't rely on this
> anymore (the issue was caught by running the pread tests in i-g-t).

I was assuming you were working on this because you were planning on
building something that *used* this swizzling.  We removed all the
userland because we never got it to actually work.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 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

Re: [PATCH 9/9] drm/i915: swizzling support for snb/ivb

[Daniel Vetter]

On Fri, Nov 11, 2011 at 20:37, Eric Anholt <eric@anholt.net> wrote:
> I was assuming you were working on this because you were planning on
> building something that *used* this swizzling.  We removed all the
> userland because we never got it to actually work.

I have started to look into the swizzle test because we absolutely
need correct swizzle information for swapout/in. Doing a pread/pwrite
instead of forcing the machine to swap simply gets the results  much
quicker (but I plan to have a separate swap test, too). While
wrestling around with swizzling I've just added in the gen6+ stuff
here, which I've noticed a while back on bspec review. And it indeed
seems to speed things up a wee bit.

I certainly don't plan to use this swizzle information in real
userspace clients ;-) but hopefully this work leads to a more correct
swap code.

Cheers, Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

Re: [PATCH 9/9] drm/i915: swizzling support for snb/ivb

[Eric Anholt]

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

On Fri, 11 Nov 2011 20:51:35 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Nov 11, 2011 at 20:37, Eric Anholt <eric@anholt.net> wrote:
> > I was assuming you were working on this because you were planning on
> > building something that *used* this swizzling.  We removed all the
> > userland because we never got it to actually work.
> 
> I have started to look into the swizzle test because we absolutely
> need correct swizzle information for swapout/in. Doing a pread/pwrite
> instead of forcing the machine to swap simply gets the results  much
> quicker (but I plan to have a separate swap test, too). While
> wrestling around with swizzling I've just added in the gen6+ stuff
> here, which I've noticed a while back on bspec review. And it indeed
> seems to speed things up a wee bit.
> 
> I certainly don't plan to use this swizzle information in real
> userspace clients ;-) but hopefully this work leads to a more correct
> swap code.

Oh, yeah, swap.  Good point, sounds like a plan.  If you manage to get
reliable swapping of tiled data on 945g, I'll owe you all the beers.

(But not if you use the blitter to untile for swapping or something.
That would just be cheating)

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 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

Re: [PATCH 9/9] drm/i915: swizzling support for snb/ivb

[Daniel Vetter]

On Fri, Nov 11, 2011 at 20:58, Eric Anholt <eric@anholt.net> wrote:
> Oh, yeah, swap.  Good point, sounds like a plan.  If you manage to get
> reliable swapping of tiled data on 945g, I'll owe you all the beers.

If you mean i945G as in desktop variant, patch 6/9 should fix the
swizzle detection on that one - it can do bit17 swizzling, too. I'd be
interested in whether this fixes all the swap issues, if you have that
machine still around ...

But as I've said there's also the gm45 bug which looks like a part of
the main memory in not swizzled. On that topic: Do you still remember
details about that machine where parts of the memory can be
unswizzled?
-Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

Re: [PATCH 9/9] drm/i915: swizzling support for snb/ivb

[Eric Anholt]

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

On Fri, 11 Nov 2011 21:18:31 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Nov 11, 2011 at 20:58, Eric Anholt <eric@anholt.net> wrote:
> > Oh, yeah, swap.  Good point, sounds like a plan.  If you manage to get
> > reliable swapping of tiled data on 945g, I'll owe you all the beers.
> 
> If you mean i945G as in desktop variant, patch 6/9 should fix the
> swizzle detection on that one - it can do bit17 swizzling, too. I'd be
> interested in whether this fixes all the swap issues, if you have that
> machine still around ...
> 
> But as I've said there's also the gm45 bug which looks like a part of
> the main memory in not swizzled. On that topic: Do you still remember
> details about that machine where parts of the memory can be
> unswizzled?

The management engine/mismatched dimms issue making "L-shaped" memory
(part dual channel, part single channel) in the 945-gm45 era was never
handled (somewhere in g45-ilk era, there was a reg the BIOS could set
that would make L-shaped memory have consistent swizzling anyway).  I
never found precise docs on how to figure out what the shape of the L
was -- I suspect you'd get to find out by poking at things like TOLUD
and experimenting with binding all the pages you can grab to find which
are swizzled.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 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

[PATCH] drm/i915: rework dev->first_error locking

[Daniel Vetter]

- reduce the irq disabled section, even for a debugfs file this was
  way too long.
- always disable irqs when taking the lock.

v2: Thou shalt not mistake locking for reference counting, so:
- reference count the error_state to protect from concurent freeeing.
  This will be only really used in the next patch.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   13 ++++++++-----
 drivers/gpu/drm/i915/i915_drv.h     |    4 ++++
 drivers/gpu/drm/i915/i915_irq.c     |   17 ++++++++++-------
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a4dfab4..84ebbd6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -769,12 +769,16 @@ static int i915_error_state(struct seq_file *m, void *unused)
 	int i, page, offset, elt;
 
 	spin_lock_irqsave(&dev_priv->error_lock, flags);
-	if (!dev_priv->first_error) {
+	error = dev_priv->first_error;
+	if (error)
+		kref_get(&error->ref);
+	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+
+	if (!error) {
 		seq_printf(m, "no error state collected\n");
-		goto out;
+		return 0;
 	}
 
-	error = dev_priv->first_error;
 
 	seq_printf(m, "Time: %ld s %ld us\n", error->time.tv_sec,
 		   error->time.tv_usec);
@@ -845,8 +849,7 @@ static int i915_error_state(struct seq_file *m, void *unused)
 	if (error->display)
 		intel_display_print_error_state(m, dev, error->display);
 
-out:
-	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
+	kref_put(&error->ref, i915_error_state_free);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7be1474..005a062 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -37,6 +37,7 @@
 #include <linux/i2c.h>
 #include <drm/intel-gtt.h>
 #include <linux/backlight.h>
+#include <linux/kref.h>
 
 /* General customization:
  */
@@ -149,6 +150,7 @@ struct sdvo_device_mapping {
 struct intel_display_error_state;
 
 struct drm_i915_error_state {
+	struct kref ref;
 	u32 eir;
 	u32 pgtbl_er;
 	u32 pipestat[I915_MAX_PIPES];
@@ -387,6 +389,7 @@ typedef struct drm_i915_private {
 	unsigned int fsb_freq, mem_freq, is_ddr3;
 
 	spinlock_t error_lock;
+	/* Protected by dev->error_lock. */
 	struct drm_i915_error_state *first_error;
 	struct work_struct error_work;
 	struct completion error_completion;
@@ -1050,6 +1053,7 @@ extern int i915_vblank_pipe_get(struct drm_device *dev, void *data,
 				struct drm_file *file_priv);
 extern int i915_vblank_swap(struct drm_device *dev, void *data,
 			    struct drm_file *file_priv);
+void i915_error_state_free(struct kref *error_ref);
 
 void
 i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a04d606..17f3ded 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -764,10 +764,11 @@ i915_error_object_free(struct drm_i915_error_object *obj)
 	kfree(obj);
 }
 
-static void
-i915_error_state_free(struct drm_device *dev,
-		      struct drm_i915_error_state *error)
+void
+i915_error_state_free(struct kref *error_ref)
 {
+	struct drm_i915_error_state *error = container_of(error_ref,
+							  typeof(*error), ref);
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(error->batchbuffer); i++)
@@ -941,6 +942,7 @@ static void i915_capture_error_state(struct drm_device *dev)
 	DRM_INFO("capturing error event; look for more information in /debug/dri/%d/i915_error_state\n",
 		 dev->primary->index);
 
+	kref_init(&error->ref);
 	error->eir = I915_READ(EIR);
 	error->pgtbl_er = I915_READ(PGTBL_ER);
 	for_each_pipe(pipe)
@@ -1017,21 +1019,22 @@ static void i915_capture_error_state(struct drm_device *dev)
 	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
 
 	if (error)
-		i915_error_state_free(dev, error);
+		i915_error_state_free(&error->ref);
 }
 
 void i915_destroy_error_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_error_state *error;
+	unsigned long flags;
 
-	spin_lock(&dev_priv->error_lock);
+	spin_lock_irqsave(&dev_priv->error_lock, flags);
 	error = dev_priv->first_error;
 	dev_priv->first_error = NULL;
-	spin_unlock(&dev_priv->error_lock);
+	spin_unlock_irqrestore(&dev_priv->error_lock, flags);
 
 	if (error)
-		i915_error_state_free(dev, error);
+		kref_put(&error->ref, i915_error_state_free);
 }
 #else
 #define i915_capture_error_state(x)
-- 
1.7.6.3

[PATCH] drm/i915: add interface to simulate gpu hangs

[Daniel Vetter]

gpu reset is a very important piece of our infrastructure.
Unfortunately we only really it test by actually hanging the gpu,
which often has bad side-effects for the entire system. And the gpu
hang handling code is one of the rather complicated pieces of code we
have, consisting of
- hang detection
- error capture
- actual gpu reset
- reset of all the gem bookkeeping
- reinitialition of the entire gpu

This patch adds a debugfs to selectively stopping rings by ceasing to
update the hw tail pointer, which will result in the gpu no longer
updating it's head pointer and eventually to the hangcheck firing.
This way we can exercise the gpu hang code under controlled conditions
without a dying gpu taking down the entire systems.

Patch motivated by me forgetting to properly reinitialize ppgtt after
a gpu reset.

Usage:

echo $((1 << $ringnum)) > i915_ring_stop # stops one ring

echo 0xffffffff > i915_ring_stop # stops all, future-proof version

then run whatever testload is desired. i915_ring_stop automatically
resets after a gpu hang is detected to avoid hanging the gpu to fast
and declaring it wedged.

v2: Incorporate feedback from Chris Wilson.

v3: Add the missing cleanup.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |   65 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.c         |    2 +
 drivers/gpu/drm/i915/i915_drv.h         |    2 +
 drivers/gpu/drm/i915/intel_ringbuffer.c |    4 ++
 4 files changed, 73 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index db83552..85328f7 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1397,6 +1397,64 @@ static const struct file_operations i915_wedged_fops = {
 };
 
 static ssize_t
+i915_ring_stop_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),
+		       "%d\n", dev_priv->stop_rings);
+
+	if (len > sizeof(buf))
+		len = sizeof(buf);
+
+	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
+}
+
+static ssize_t
+i915_ring_stop_write(struct file *filp,
+		     const char __user *ubuf,
+		     size_t cnt,
+		     loff_t *ppos)
+{
+	struct drm_device *dev = filp->private_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	char buf[20];
+	int val = 0;
+
+	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("Stopping rings %u\n", val);
+
+	mutex_lock(&dev->struct_mutex);
+	dev_priv->stop_rings = val;
+	mutex_unlock(&dev->struct_mutex);
+
+	return cnt;
+}
+
+static const struct file_operations i915_ring_stop_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_debugfs_common_open,
+	.read = i915_ring_stop_read,
+	.write = i915_ring_stop_write,
+	.llseek = default_llseek,
+};
+static ssize_t
 i915_max_freq_read(struct file *filp,
 		   char __user *ubuf,
 		   size_t max,
@@ -1701,6 +1759,11 @@ int i915_debugfs_init(struct drm_minor *minor)
 				  &i915_cache_sharing_fops);
 	if (ret)
 		return ret;
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_ring_stop",
+				  &i915_ring_stop_fops);
+	if (ret)
+		return ret;
 
 	return drm_debugfs_create_files(i915_debugfs_list,
 					I915_DEBUGFS_ENTRIES,
@@ -1719,6 +1782,8 @@ void i915_debugfs_cleanup(struct drm_minor *minor)
 				 1, minor);
 	drm_debugfs_remove_files((struct drm_info_list *) &i915_cache_sharing_fops,
 				 1, minor);
+	drm_debugfs_remove_files((struct drm_info_list *) &i915_ring_stop_fops,
+				 1, minor);
 }
 
 #endif /* CONFIG_DEBUG_FS */
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4a2eb68..6dd219b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -638,6 +638,8 @@ int i915_reset(struct drm_device *dev, u8 flags)
 	if (!mutex_trylock(&dev->struct_mutex))
 		return -EBUSY;
 
+	dev_priv->stop_rings = 0;
+
 	i915_gem_reset(dev);
 
 	ret = -ENODEV;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5abc828..621349e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -346,6 +346,8 @@ typedef struct drm_i915_private {
 	uint32_t last_instdone;
 	uint32_t last_instdone1;
 
+	unsigned int stop_rings;
+
 	unsigned long cfb_size;
 	unsigned int cfb_fb;
 	enum plane cfb_plane;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2d476a9..cf6e159 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1179,7 +1179,11 @@ int intel_ring_begin(struct intel_ring_buffer *ring,
 
 void intel_ring_advance(struct intel_ring_buffer *ring)
 {
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
 	ring->tail &= ring->size - 1;
+	if (dev_priv->stop_rings & intel_ring_flag(ring))
+		return;
 	ring->write_tail(ring, ring->tail);
 }
 
-- 
1.7.7.3

Re: [PATCH] drm/i915: add interface to simulate gpu hangs

[Chris Wilson]

On Fri,  2 Dec 2011 23:21:49 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> gpu reset is a very important piece of our infrastructure.
> Unfortunately we only really it test by actually hanging the gpu,
> which often has bad side-effects for the entire system. And the gpu
> hang handling code is one of the rather complicated pieces of code we
> have, consisting of
> - hang detection
> - error capture
> - actual gpu reset
> - reset of all the gem bookkeeping
> - reinitialition of the entire gpu
> 
> This patch adds a debugfs to selectively stopping rings by ceasing to
> update the hw tail pointer, which will result in the gpu no longer
> updating it's head pointer and eventually to the hangcheck firing.
> This way we can exercise the gpu hang code under controlled conditions
> without a dying gpu taking down the entire systems.
> 
> Patch motivated by me forgetting to properly reinitialize ppgtt after
> a gpu reset.
> 
> Usage:
> 
> echo $((1 << $ringnum)) > i915_ring_stop # stops one ring
> 
> echo 0xffffffff > i915_ring_stop # stops all, future-proof version
> 
> then run whatever testload is desired. i915_ring_stop automatically
> resets after a gpu hang is detected to avoid hanging the gpu to fast
> and declaring it wedged.
> 
> v2: Incorporate feedback from Chris Wilson.
> 
> v3: Add the missing cleanup.

I think I've made my peace with this patch. I'm still not completely
sold on its value, but if Daniel found it useful then it has merit.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c     |   65 +++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.c         |    2 +
>  drivers/gpu/drm/i915/i915_drv.h         |    2 +
>  drivers/gpu/drm/i915/intel_ringbuffer.c |    4 ++
>  4 files changed, 73 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index db83552..85328f7 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1397,6 +1397,64 @@ static const struct file_operations i915_wedged_fops = {
>  };
>  
>  static ssize_t
> +i915_ring_stop_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),
> +		       "%d\n", dev_priv->stop_rings);
%08x since it is a flags value, though 8 may be overkill! 

> +
> +	if (len > sizeof(buf))
> +		len = sizeof(buf);
> +
> +	return simple_read_from_buffer(ubuf, max, ppos, buf, len);
> +}
> +
> +static ssize_t
> +i915_ring_stop_write(struct file *filp,
> +		     const char __user *ubuf,
> +		     size_t cnt,
> +		     loff_t *ppos)
> +{
> +	struct drm_device *dev = filp->private_data;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	char buf[20];
> +	int val = 0;
> +
> +	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("Stopping rings %u\n", val);
%x here as well

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: add interface to simulate gpu hangs

[Ben Widawsky]

On Fri, Dec 02, 2011 at 11:21:49PM +0100, Daniel Vetter wrote:
> gpu reset is a very important piece of our infrastructure.
> Unfortunately we only really it test by actually hanging the gpu,
> which often has bad side-effects for the entire system. And the gpu
> hang handling code is one of the rather complicated pieces of code we
> have, consisting of
> - hang detection
> - error capture
> - actual gpu reset
> - reset of all the gem bookkeeping
> - reinitialition of the entire gpu
> 
> This patch adds a debugfs to selectively stopping rings by ceasing to
> update the hw tail pointer, which will result in the gpu no longer
> updating it's head pointer and eventually to the hangcheck firing.
> This way we can exercise the gpu hang code under controlled conditions
> without a dying gpu taking down the entire systems.
> 
> Patch motivated by me forgetting to properly reinitialize ppgtt after
> a gpu reset.
> 
> Usage:
> 
> echo $((1 << $ringnum)) > i915_ring_stop # stops one ring
> 
> echo 0xffffffff > i915_ring_stop # stops all, future-proof version
> 
> then run whatever testload is desired. i915_ring_stop automatically
> resets after a gpu hang is detected to avoid hanging the gpu to fast
> and declaring it wedged.
> 
> v2: Incorporate feedback from Chris Wilson.
> 
> v3: Add the missing cleanup.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Ben Widawsky <ben@bwidawsk.net>