GPU lockup dumping

GPU lockup dumping

[j.glisse]

So here is improved patchset, where i splited ground work necessary
for the dumping into their own patch. The debugfs improvement could
probably be usefull to intel instead of having i915 have it's own
debugfs file stuff.

The lockup dumping public api have been move into radeon_drm.h

Stressing the fact again that dump are self contained ie they have
all the data needed to be replayed (vertex, indices, shader, texture,
...).

Would really like to get this into 3.5, the new API is pretty much
straightforward and userspace tools can easily be made to convert
it to other format. The change to the driver is self contained.

Cheers,
Jerome

[PATCH 1/5] drm/debugfs: allow driver to provide a custom read callback

[j.glisse]

From: Jerome Glisse <jglisse@redhat.com>

Allow driver to provide a custom read callback for debugfs file.
Usefull if driver try to dump big buffer, avoid double buffering.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/drm_debugfs.c             |   19 ++++++++++++++++---
 drivers/gpu/drm/i915/i915_debugfs.c       |    2 +-
 drivers/gpu/drm/nouveau/nouveau_debugfs.c |    4 ++--
 drivers/gpu/drm/radeon/radeon_device.c    |    4 ++--
 include/drm/drmP.h                        |    8 +++++++-
 5 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 1c7a1c0..d5194e6 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -63,11 +63,22 @@ static int drm_debugfs_open(struct inode *inode, struct file *file)
 	return single_open(file, node->info_ent->show, node);
 }
 
+static ssize_t drm_debugfs_read(struct file *filp, char __user *ubuf,
+				size_t max, loff_t *ppos)
+{
+	struct seq_file *m = filp->private_data;
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+
+	if (node->read) {
+		return node->read(filp, ubuf, max, ppos);
+	}
+	return seq_read(filp, ubuf, max, ppos);
+}
 
 static const struct file_operations drm_debugfs_fops = {
 	.owner = THIS_MODULE,
 	.open = drm_debugfs_open,
-	.read = seq_read,
+	.read = drm_debugfs_read,
 	.llseek = seq_lseek,
 	.release = single_release,
 };
@@ -86,7 +97,8 @@ static const struct file_operations drm_debugfs_fops = {
  * gdm_debugfs_lists in the given root directory.
  */
 int drm_debugfs_create_files(struct drm_info_list *files, int count,
-			     struct dentry *root, struct drm_minor *minor)
+			     struct dentry *root, struct drm_minor *minor,
+			     drm_debugfs_read_t read)
 {
 	struct drm_device *dev = minor->dev;
 	struct dentry *ent;
@@ -118,6 +130,7 @@ int drm_debugfs_create_files(struct drm_info_list *files, int count,
 		tmp->minor = minor;
 		tmp->dent = ent;
 		tmp->info_ent = &files[i];
+		tmp->read = read;
 
 		mutex_lock(&minor->debugfs_lock);
 		list_add(&tmp->list, &minor->debugfs_list);
@@ -159,7 +172,7 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 	}
 
 	ret = drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
-				       minor->debugfs_root, minor);
+				       minor->debugfs_root, minor, NULL);
 	if (ret) {
 		debugfs_remove(minor->debugfs_root);
 		minor->debugfs_root = NULL;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 950f72a..636c4f4 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2018,7 +2018,7 @@ int i915_debugfs_init(struct drm_minor *minor)
 
 	return drm_debugfs_create_files(i915_debugfs_list,
 					I915_DEBUGFS_ENTRIES,
-					minor->debugfs_root, minor);
+					minor->debugfs_root, minor, NULL);
 }
 
 void i915_debugfs_cleanup(struct drm_minor *minor)
diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index fa2ec49..12cbc5c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -94,7 +94,7 @@ nouveau_debugfs_channel_init(struct nouveau_channel *chan)
 
 	ret = drm_debugfs_create_files(&chan->debugfs.info, 1,
 				       dev_priv->debugfs.channel_root,
-				       chan->dev->primary);
+				       chan->dev->primary, NULL);
 	if (ret == 0)
 		chan->debugfs.active = true;
 	return ret;
@@ -186,7 +186,7 @@ int
 nouveau_debugfs_init(struct drm_minor *minor)
 {
 	drm_debugfs_create_files(nouveau_debugfs_list, NOUVEAU_DEBUGFS_ENTRIES,
-				 minor->debugfs_root, minor);
+				 minor->debugfs_root, minor, NULL);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 3d41525..944ac11 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1039,10 +1039,10 @@ int radeon_debugfs_add_files(struct radeon_device *rdev,
 #if defined(CONFIG_DEBUG_FS)
 	drm_debugfs_create_files(files, nfiles,
 				 rdev->ddev->control->debugfs_root,
-				 rdev->ddev->control);
+				 rdev->ddev->control, NULL);
 	drm_debugfs_create_files(files, nfiles,
 				 rdev->ddev->primary->debugfs_root,
-				 rdev->ddev->primary);
+				 rdev->ddev->primary, NULL);
 #endif
 	return 0;
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index efd1249..1e41407 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1011,6 +1011,8 @@ struct drm_info_node {
 	struct drm_minor *minor;
 	struct drm_info_list *info_ent;
 	struct dentry *dent;
+	/* read callback */
+	ssize_t (*read)(struct file *filp, char __user *ubuf,size_t max, loff_t *ppos);
 };
 
 /**
@@ -1528,10 +1530,14 @@ extern int drm_proc_cleanup(struct drm_minor *minor, struct proc_dir_entry *root
 
 				/* Debugfs support */
 #if defined(CONFIG_DEBUG_FS)
+typedef ssize_t (drm_debugfs_read_t)(struct file *filp, char __user *ubuf,
+				     size_t max, loff_t *ppos);
+
 extern int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 			    struct dentry *root);
 extern int drm_debugfs_create_files(struct drm_info_list *files, int count,
-				    struct dentry *root, struct drm_minor *minor);
+				    struct dentry *root, struct drm_minor *minor,
+				    drm_debugfs_read_t read);
 extern int drm_debugfs_remove_files(struct drm_info_list *files, int count,
                                     struct drm_minor *minor);
 extern int drm_debugfs_cleanup(struct drm_minor *minor);
-- 
1.7.7.6

[PATCH 2/5] drm/radeon: allow radeon debugfs helper to provide custom read

[j.glisse]

From: Jerome Glisse <jglisse@redhat.com>

Allow radeon debugfs file to provide a custom read function. This
is usefull in case you don't want to double buffer with seq_file,
or simply in case the buffer data is too big to be buffered by
seq_file.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/r100.c          |    6 +++---
 drivers/gpu/drm/radeon/r300.c          |    2 +-
 drivers/gpu/drm/radeon/r420.c          |    2 +-
 drivers/gpu/drm/radeon/r600.c          |    2 +-
 drivers/gpu/drm/radeon/radeon.h        |    3 ++-
 drivers/gpu/drm/radeon/radeon_device.c |    7 ++++---
 drivers/gpu/drm/radeon/radeon_fence.c  |    2 +-
 drivers/gpu/drm/radeon/radeon_pm.c     |    2 +-
 drivers/gpu/drm/radeon/radeon_ring.c   |    4 ++--
 drivers/gpu/drm/radeon/radeon_ttm.c    |    2 +-
 drivers/gpu/drm/radeon/rs400.c         |    2 +-
 drivers/gpu/drm/radeon/rv515.c         |    4 ++--
 12 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index 0874a6d..8ed365f 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -2694,7 +2694,7 @@ static struct drm_info_list r100_debugfs_mc_info_list[] = {
 int r100_debugfs_rbbm_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, r100_debugfs_rbbm_list, 1);
+	return radeon_debugfs_add_files(rdev, r100_debugfs_rbbm_list, 1, NULL);
 #else
 	return 0;
 #endif
@@ -2703,7 +2703,7 @@ int r100_debugfs_rbbm_init(struct radeon_device *rdev)
 int r100_debugfs_cp_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, r100_debugfs_cp_list, 2);
+	return radeon_debugfs_add_files(rdev, r100_debugfs_cp_list, 2, NULL);
 #else
 	return 0;
 #endif
@@ -2712,7 +2712,7 @@ int r100_debugfs_cp_init(struct radeon_device *rdev)
 int r100_debugfs_mc_info_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, r100_debugfs_mc_info_list, 1);
+	return radeon_debugfs_add_files(rdev, r100_debugfs_mc_info_list, 1, NULL);
 #else
 	return 0;
 #endif
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index 97722a3..edb9eeb 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -586,7 +586,7 @@ static struct drm_info_list rv370_pcie_gart_info_list[] = {
 static int rv370_debugfs_pcie_gart_info_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, rv370_pcie_gart_info_list, 1);
+	return radeon_debugfs_add_files(rdev, rv370_pcie_gart_info_list, 1, NULL);
 #else
 	return 0;
 #endif
diff --git a/drivers/gpu/drm/radeon/r420.c b/drivers/gpu/drm/radeon/r420.c
index 99137be..4eddcfc 100644
--- a/drivers/gpu/drm/radeon/r420.c
+++ b/drivers/gpu/drm/radeon/r420.c
@@ -491,7 +491,7 @@ static struct drm_info_list r420_pipes_info_list[] = {
 int r420_debugfs_pipes_info_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, r420_pipes_info_list, 1);
+	return radeon_debugfs_add_files(rdev, r420_pipes_info_list, 1, NULL);
 #else
 	return 0;
 #endif
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 4c0d8c9..afc458a 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -3589,7 +3589,7 @@ static struct drm_info_list r600_mc_info_list[] = {
 int r600_debugfs_mc_info_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, r600_mc_info_list, ARRAY_SIZE(r600_mc_info_list));
+	return radeon_debugfs_add_files(rdev, r600_mc_info_list, ARRAY_SIZE(r600_mc_info_list), NULL);
 #else
 	return 0;
 #endif
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 9783178..2aa0dfa 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1128,7 +1128,8 @@ struct radeon_debugfs {
 
 int radeon_debugfs_add_files(struct radeon_device *rdev,
 			     struct drm_info_list *files,
-			     unsigned nfiles);
+			     unsigned nfiles,
+			     drm_debugfs_read_t read);
 int radeon_debugfs_fence_init(struct radeon_device *rdev);
 
 
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 944ac11..b5f4fb9 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1015,7 +1015,8 @@ int radeon_gpu_reset(struct radeon_device *rdev)
  */
 int radeon_debugfs_add_files(struct radeon_device *rdev,
 			     struct drm_info_list *files,
-			     unsigned nfiles)
+			     unsigned nfiles,
+			     drm_debugfs_read_t read)
 {
 	unsigned i;
 
@@ -1039,10 +1040,10 @@ int radeon_debugfs_add_files(struct radeon_device *rdev,
 #if defined(CONFIG_DEBUG_FS)
 	drm_debugfs_create_files(files, nfiles,
 				 rdev->ddev->control->debugfs_root,
-				 rdev->ddev->control, NULL);
+				 rdev->ddev->control, read);
 	drm_debugfs_create_files(files, nfiles,
 				 rdev->ddev->primary->debugfs_root,
-				 rdev->ddev->primary, NULL);
+				 rdev->ddev->primary, read);
 #endif
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 11f5f40..b4979c9 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -618,7 +618,7 @@ static struct drm_info_list radeon_debugfs_fence_list[] = {
 int radeon_debugfs_fence_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, radeon_debugfs_fence_list, 1);
+	return radeon_debugfs_add_files(rdev, radeon_debugfs_fence_list, 1, NULL);
 #else
 	return 0;
 #endif
diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
index 0882554..ee4fd15 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -876,7 +876,7 @@ static struct drm_info_list radeon_pm_info_list[] = {
 static int radeon_debugfs_pm_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, radeon_pm_info_list, ARRAY_SIZE(radeon_pm_info_list));
+	return radeon_debugfs_add_files(rdev, radeon_pm_info_list, ARRAY_SIZE(radeon_pm_info_list), NULL);
 #else
 	return 0;
 #endif
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index a5dee76..7352b76 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -515,7 +515,7 @@ int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *rin
 		if (&rdev->ring[ridx] != ring)
 			continue;
 
-		r = radeon_debugfs_add_files(rdev, info, 1);
+		r = radeon_debugfs_add_files(rdev, info, 1, NULL);
 		if (r)
 			return r;
 	}
@@ -526,7 +526,7 @@ int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *rin
 int radeon_debugfs_sa_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, radeon_debugfs_sa_list, 1);
+	return radeon_debugfs_add_files(rdev, radeon_debugfs_sa_list, 1, NULL);
 #else
 	return 0;
 #endif
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 0f6aee8..b01c3c6 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -903,7 +903,7 @@ static int radeon_ttm_debugfs_init(struct radeon_device *rdev)
 		radeon_mem_types_list[i++].data = NULL;
 	}
 #endif
-	return radeon_debugfs_add_files(rdev, radeon_mem_types_list, i);
+	return radeon_debugfs_add_files(rdev, radeon_mem_types_list, i, NULL);
 
 #endif
 	return 0;
diff --git a/drivers/gpu/drm/radeon/rs400.c b/drivers/gpu/drm/radeon/rs400.c
index a464eb5..b135601 100644
--- a/drivers/gpu/drm/radeon/rs400.c
+++ b/drivers/gpu/drm/radeon/rs400.c
@@ -364,7 +364,7 @@ static struct drm_info_list rs400_gart_info_list[] = {
 static int rs400_debugfs_pcie_gart_info_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, rs400_gart_info_list, 1);
+	return radeon_debugfs_add_files(rdev, rs400_gart_info_list, 1, NULL);
 #else
 	return 0;
 #endif
diff --git a/drivers/gpu/drm/radeon/rv515.c b/drivers/gpu/drm/radeon/rv515.c
index 7f08ced..9a74cf2 100644
--- a/drivers/gpu/drm/radeon/rv515.c
+++ b/drivers/gpu/drm/radeon/rv515.c
@@ -264,7 +264,7 @@ static struct drm_info_list rv515_ga_info_list[] = {
 int rv515_debugfs_pipes_info_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, rv515_pipes_info_list, 1);
+	return radeon_debugfs_add_files(rdev, rv515_pipes_info_list, 1, NULL);
 #else
 	return 0;
 #endif
@@ -273,7 +273,7 @@ int rv515_debugfs_pipes_info_init(struct radeon_device *rdev)
 int rv515_debugfs_ga_info_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, rv515_ga_info_list, 1);
+	return radeon_debugfs_add_files(rdev, rv515_ga_info_list, 1, NULL);
 #else
 	return 0;
 #endif
-- 
1.7.7.6

[PATCH 3/5] drm/radeon: allow radeon_vm_bo_update_pte caller to get bo virtual offset

[j.glisse]

From: Jerome Glisse <jglisse@redhat.com>

Allow caller of radeon_vm_bo_update_pte to get the virtual bo offset.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h      |    3 ++-
 drivers/gpu/drm/radeon/radeon_cs.c   |    2 +-
 drivers/gpu/drm/radeon/radeon_gart.c |   11 ++++++++---
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 2aa0dfa..dc51ee9 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1794,7 +1794,8 @@ void radeon_vm_unbind(struct radeon_device *rdev, struct radeon_vm *vm);
 int radeon_vm_bo_update_pte(struct radeon_device *rdev,
 			    struct radeon_vm *vm,
 			    struct radeon_bo *bo,
-			    struct ttm_mem_reg *mem);
+			    struct ttm_mem_reg *mem,
+			    uint64_t *vm_offset);
 void radeon_vm_bo_invalidate(struct radeon_device *rdev,
 			     struct radeon_bo *bo);
 int radeon_vm_bo_add(struct radeon_device *rdev,
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index c7d64a7..e86907a 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -386,7 +386,7 @@ static int radeon_bo_vm_update_pte(struct radeon_cs_parser *parser,
 
 	list_for_each_entry(lobj, &parser->validated, tv.head) {
 		bo = lobj->bo;
-		r = radeon_vm_bo_update_pte(parser->rdev, vm, bo, &bo->tbo.mem);
+		r = radeon_vm_bo_update_pte(parser->rdev, vm, bo, &bo->tbo.mem, NULL);
 		if (r) {
 			return r;
 		}
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 8e9ef34..f91a95d 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -433,7 +433,7 @@ retry_id:
 	vm->id = id;
 	list_add_tail(&vm->list, &rdev->vm_manager.lru_vm);
 	return radeon_vm_bo_update_pte(rdev, vm, rdev->ring_tmp_bo.bo,
-				       &rdev->ring_tmp_bo.bo->tbo.mem);
+				       &rdev->ring_tmp_bo.bo->tbo.mem, NULL);
 }
 
 /* object have to be reserved */
@@ -540,7 +540,8 @@ static u64 radeon_vm_get_addr(struct radeon_device *rdev,
 int radeon_vm_bo_update_pte(struct radeon_device *rdev,
 			    struct radeon_vm *vm,
 			    struct radeon_bo *bo,
-			    struct ttm_mem_reg *mem)
+			    struct ttm_mem_reg *mem,
+			    uint64_t *vm_offset)
 {
 	struct radeon_bo_va *bo_va;
 	unsigned ngpu_pages, i;
@@ -560,6 +561,10 @@ int radeon_vm_bo_update_pte(struct radeon_device *rdev,
 	if (bo_va->valid)
 		return 0;
 
+	if (vm_offset) {
+		*vm_offset = bo_va->soffset;
+	}
+
 	ngpu_pages = radeon_bo_ngpu_pages(bo);
 	bo_va->flags &= ~RADEON_VM_PAGE_VALID;
 	bo_va->flags &= ~RADEON_VM_PAGE_SYSTEM;
@@ -597,7 +602,7 @@ int radeon_vm_bo_rmv(struct radeon_device *rdev,
 
 	mutex_lock(&vm->mutex);
 	radeon_mutex_lock(&rdev->cs_mutex);
-	radeon_vm_bo_update_pte(rdev, vm, bo, NULL);
+	radeon_vm_bo_update_pte(rdev, vm, bo, NULL, NULL);
 	radeon_mutex_unlock(&rdev->cs_mutex);
 	list_del(&bo_va->vm_list);
 	mutex_unlock(&vm->mutex);
-- 
1.7.7.6

[PATCH 4/5] drm/radeon: add lockup faulty command recording v2

[j.glisse]

From: Jerome Glisse <jglisse@redhat.com>

This try to identify the faulty user command stream that caused
lockup. If it finds one it create big blob that contains all
information, this include packet stream but also snapshot of all
bo used by the faulty packet stream.

This means that the blod is self contained and can be fully
replayed.

v2: Better commit message. Split out the radeon debugfs change
    into its own patch. Split out the vm offset change into its
    own patch. Add data buffer flags so kernel can flags
    bo that are dumped with valid data. Remove the family from
    the header and instead let the userspace tools rely on the
    pci id. Avoid doing whitespace/indentation cleaning.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h        |   15 ++++
 drivers/gpu/drm/radeon/radeon_cs.c     |   68 +++++++++++++++++--
 drivers/gpu/drm/radeon/radeon_device.c |   16 ++++
 drivers/gpu/drm/radeon/radeon_object.h |   11 +++-
 drivers/gpu/drm/radeon/radeon_ring.c   |   18 +++++-
 drivers/gpu/drm/radeon/radeon_sa.c     |  119 ++++++++++++++++++++++++++++++++
 include/drm/radeon_drm.h               |  102 +++++++++++++++++++++++++++
 7 files changed, 341 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index dc51ee9..f27fca6 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -74,6 +74,7 @@
 #include "radeon_family.h"
 #include "radeon_mode.h"
 #include "radeon_reg.h"
+#include "radeon_drm.h"
 
 /*
  * Modules parameters.
@@ -395,6 +396,11 @@ struct radeon_sa_manager {
 
 struct radeon_sa_bo;
 
+struct radeon_dump {
+	struct rati_data_buffer		buffer;
+	struct radeon_bo		*bo;
+};
+
 /* sub-allocation buffer */
 struct radeon_sa_bo {
 	struct list_head		olist;
@@ -403,6 +409,8 @@ struct radeon_sa_bo {
 	unsigned			soffset;
 	unsigned			eoffset;
 	struct radeon_fence		*fence;
+	unsigned			nbuffers;
+	struct radeon_dump		*buffers;
 };
 
 /*
@@ -846,6 +854,8 @@ struct radeon_cs_parser {
 	u32			cs_flags;
 	u32			ring;
 	s32			priority;
+	unsigned		nbuffers;
+	struct radeon_dump	*buffers;
 };
 
 extern int radeon_cs_update_pages(struct radeon_cs_parser *p, int pg_idx);
@@ -1548,6 +1558,11 @@ struct radeon_device {
 	unsigned 		debugfs_count;
 	/* virtual memory */
 	struct radeon_vm_manager	vm_manager;
+	/* lockup blob dumping */
+	unsigned			blob_dump;
+	struct rati_header		blob_header;
+	uint64_t			blob_size;
+	void				*blob;
 };
 
 int radeon_device_init(struct radeon_device *rdev,
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index e86907a..2b4a3ca 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -36,7 +36,7 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
 {
 	struct drm_device *ddev = p->rdev->ddev;
 	struct radeon_cs_chunk *chunk;
-	unsigned i, j;
+	unsigned i, j, ib;
 	bool duplicate;
 
 	if (p->chunk_relocs_idx == -1) {
@@ -53,7 +53,16 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
 	if (p->relocs == NULL) {
 		return -ENOMEM;
 	}
-	for (i = 0; i < p->nrelocs; i++) {
+	p->buffers = NULL;
+	p->nbuffers = 0;
+	if (p->rdev->blob_dump) {
+		p->buffers = kcalloc(p->nrelocs, sizeof(*p->buffers), GFP_KERNEL);
+		if (p->buffers == NULL) {
+			return -ENOMEM;
+		}
+		p->nbuffers = p->nrelocs;
+	}
+	for (i = 0, ib = 0; i < p->nrelocs; i++) {
 		struct drm_radeon_cs_reloc *r;
 
 		duplicate = false;
@@ -85,8 +94,23 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
 			radeon_bo_list_add_object(&p->relocs[i].lobj,
 						  &p->validated);
 
-		} else
+			/* initialize dump struct */
+			if (p->rdev->blob_dump) {
+				p->buffers[ib].bo =  p->relocs[i].robj;
+				p->buffers[ib].buffer.id = RATI_DATA_BUFFER;
+				p->buffers[ib].buffer.ver = 1;
+				p->buffers[ib].buffer.size = radeon_bo_size(p->buffers[i].bo);
+				p->buffers[ib].buffer.paded_ndw = ALIGN(p->buffers[i].buffer.size >> 2, 2);
+				p->buffers[ib].buffer.alignment = radeon_bo_alignment(p->buffers[i].bo);
+				p->buffers[ib].buffer.flags = 0;
+				ib++;
+			}
+		} else {
 			p->relocs[i].handle = 0;
+			if (p->rdev->blob_dump) {
+				p->nbuffers--;
+			}
+		}
 	}
 	return radeon_bo_list_validate(&p->validated);
 }
@@ -303,11 +327,13 @@ static void radeon_cs_parser_fini(struct radeon_cs_parser *parser, int error)
 {
 	unsigned i;
 
-	if (!error)
+	if (!error) {
 		ttm_eu_fence_buffer_objects(&parser->validated,
 					    parser->ib.fence);
-	else
+	} else {
 		ttm_eu_backoff_reservation(&parser->validated);
+		kfree(parser->buffers);
+	}
 
 	if (parser->relocs != NULL) {
 		for (i = 0; i < parser->nrelocs; i++) {
@@ -335,6 +361,7 @@ static int radeon_cs_ib_chunk(struct radeon_device *rdev,
 			      struct radeon_cs_parser *parser)
 {
 	struct radeon_cs_chunk *ib_chunk;
+	unsigned i;
 	int r;
 
 	if (parser->chunk_ib_idx == -1)
@@ -370,6 +397,17 @@ static int radeon_cs_ib_chunk(struct radeon_device *rdev,
 		DRM_ERROR("Failed to synchronize rings !\n");
 	}
 	parser->ib.vm_id = 0;
+
+	/* update dump informations */
+	if (parser->rdev->blob_dump) {
+		for (i = 0; i < parser->nbuffers; i++) {
+			parser->buffers[i].buffer.offset = radeon_bo_gpu_offset(parser->buffers[i].bo);
+		}
+		parser->ib.sa_bo->buffers = parser->buffers;
+		parser->ib.sa_bo->nbuffers = parser->nbuffers;
+		parser->buffers = NULL;
+	}
+
 	r = radeon_ib_schedule(rdev, &parser->ib);
 	if (r) {
 		DRM_ERROR("Failed to schedule IB !\n");
@@ -382,14 +420,24 @@ static int radeon_bo_vm_update_pte(struct radeon_cs_parser *parser,
 {
 	struct radeon_bo_list *lobj;
 	struct radeon_bo *bo;
+	uint64_t vm_offset;
+	unsigned i;
 	int r;
 
 	list_for_each_entry(lobj, &parser->validated, tv.head) {
 		bo = lobj->bo;
-		r = radeon_vm_bo_update_pte(parser->rdev, vm, bo, &bo->tbo.mem, NULL);
+		r = radeon_vm_bo_update_pte(parser->rdev, vm, bo, &bo->tbo.mem, &vm_offset);
 		if (r) {
 			return r;
 		}
+		if (parser->rdev->blob_dump) {
+			for (i = 0; i < parser->nbuffers; i++) {
+				if (parser->buffers[i].bo == bo) {
+					parser->buffers[i].buffer.offset = vm_offset;
+					break;
+				}
+			}
+		}
 	}
 	return 0;
 }
@@ -488,6 +536,14 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev,
 	 */
 	parser->ib.gpu_addr = parser->ib.sa_bo->soffset;
 	parser->ib.is_const_ib = false;
+
+	/* update dump informations */
+	if (parser->rdev->blob_dump) {
+		parser->ib.sa_bo->buffers = parser->buffers;
+		parser->ib.sa_bo->nbuffers = parser->nbuffers;
+		parser->buffers = NULL;
+	}
+
 	r = radeon_ib_schedule(rdev, &parser->ib);
 out:
 	if (!r) {
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index b5f4fb9..ec854da 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -722,6 +722,13 @@ int radeon_device_init(struct radeon_device *rdev,
 	rdev->mc.gtt_size = radeon_gart_size * 1024 * 1024;
 	rdev->accel_working = false;
 
+	/* initializa blob dumping */
+	rdev->blob_dump = true;
+	rdev->blob_header.id = RATI_HEADER;
+	rdev->blob_header.ver = 1;
+	rdev->blob_header.pciid = rdev->pdev->vendor << 16;
+	rdev->blob_header.pciid |= rdev->pdev->device;
+
 	DRM_INFO("initializing kernel modesetting (%s 0x%04X:0x%04X 0x%04X:0x%04X).\n",
 		radeon_family_name[rdev->family], pdev->vendor, pdev->device,
 		pdev->subsystem_vendor, pdev->subsystem_device);
@@ -860,6 +867,10 @@ void radeon_device_fini(struct radeon_device *rdev)
 	iounmap(rdev->rmmio);
 	rdev->rmmio = NULL;
 	radeon_debugfs_remove_files(rdev);
+
+	rdev->blob_size = 0;
+	vfree(rdev->blob);
+	rdev->blob = NULL;
 }
 
 
@@ -987,6 +998,11 @@ int radeon_gpu_reset(struct radeon_device *rdev)
 	int r;
 	int resched;
 
+	/* FIXME we should detect which ring caused a lockup and pass
+	 * appropriate ring parameter here
+	 */
+	radeon_sa_bo_faulty(rdev, &rdev->ring_tmp_bo, RADEON_RING_TYPE_GFX_INDEX);
+
 	radeon_save_bios_scratch_regs(rdev);
 	/* block TTM */
 	resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index befec7d..265cc0c 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -93,6 +93,11 @@ static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo)
 	return (bo->tbo.mem.page_alignment << PAGE_SHIFT) / RADEON_GPU_PAGE_SIZE;
 }
 
+static inline unsigned radeon_bo_alignment(struct radeon_bo *bo)
+{
+	return (bo->tbo.mem.page_alignment << PAGE_SHIFT);
+}
+
 /**
  * radeon_bo_mmap_offset - return mmap offset of bo
  * @bo:	radeon object for which we query the offset
@@ -173,10 +178,14 @@ extern int radeon_sa_bo_new(struct radeon_device *rdev,
 extern void radeon_sa_bo_free(struct radeon_device *rdev,
 			      struct radeon_sa_bo **sa_bo,
 			      struct radeon_fence *fence);
+extern void radeon_sa_bo_faulty(struct radeon_device *rdev,
+				struct radeon_sa_manager *sa_manager,
+				unsigned ring);
 #if defined(CONFIG_DEBUG_FS)
+extern ssize_t radeon_blob_read(struct file *filp, char __user *ubuf,
+				size_t max, loff_t *ppos);
 extern void radeon_sa_bo_dump_debug_info(struct radeon_sa_manager *sa_manager,
 					 struct seq_file *m);
 #endif
 
-
 #endif
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 7352b76..6982d8f 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -497,10 +497,20 @@ static int radeon_debugfs_sa_info(struct seq_file *m, void *data)
 
 }
 
+static int radeon_debugfs_lockup(struct seq_file *m, void *data)
+{
+	/* empty, should never be call */
+	return 0;
+}
+
 static struct drm_info_list radeon_debugfs_sa_list[] = {
         {"radeon_sa_info", &radeon_debugfs_sa_info, 0, NULL},
 };
 
+static struct drm_info_list radeon_debugfs_lockup_list[] = {
+	{"radeon_lockup", &radeon_debugfs_lockup, 0, NULL},
+};
+
 #endif
 
 int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *ring)
@@ -526,7 +536,13 @@ int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *rin
 int radeon_debugfs_sa_init(struct radeon_device *rdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	return radeon_debugfs_add_files(rdev, radeon_debugfs_sa_list, 1, NULL);
+	int r;
+
+	r = radeon_debugfs_add_files(rdev, radeon_debugfs_sa_list, 1, NULL);
+	if (r) {
+		return r;
+	}
+	return radeon_debugfs_add_files(rdev, radeon_debugfs_lockup_list, 1, radeon_blob_read);
 #else
 	return 0;
 #endif
diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
index c3ac7f4..94829be 100644
--- a/drivers/gpu/drm/radeon/radeon_sa.c
+++ b/drivers/gpu/drm/radeon/radeon_sa.c
@@ -148,6 +148,7 @@ static void radeon_sa_bo_remove_locked(struct radeon_sa_bo *sa_bo)
 	list_del_init(&sa_bo->olist);
 	list_del_init(&sa_bo->flist);
 	radeon_fence_unref(&sa_bo->fence);
+	kfree(sa_bo->buffers);
 	kfree(sa_bo);
 }
 
@@ -294,6 +295,8 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
 	}
 	(*sa_bo)->manager = sa_manager;
 	(*sa_bo)->fence = NULL;
+	(*sa_bo)->buffers = NULL;
+	(*sa_bo)->nbuffers = 0;
 	INIT_LIST_HEAD(&(*sa_bo)->olist);
 	INIT_LIST_HEAD(&(*sa_bo)->flist);
 
@@ -360,7 +363,123 @@ void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo **sa_bo,
 	*sa_bo = NULL;
 }
 
+void radeon_sa_bo_faulty(struct radeon_device *rdev,
+			 struct radeon_sa_manager *sa_manager,
+			 unsigned ring)
+{
+	struct radeon_sa_bo *sa_bo = NULL, *tmp;
+	struct rati_cmd_buffer cmd;
+	unsigned long offset = 0;
+	unsigned i;
+
+	rdev->blob_size = 0;
+	vfree(rdev->blob);
+	rdev->blob = NULL;
+
+	spin_lock(&sa_manager->lock);
+	list_for_each_entry(tmp, &sa_manager->olist, olist) {
+		if (tmp->fence == NULL ||
+		    tmp->buffers == NULL ||
+		    tmp->fence->ring != ring ||
+		    tmp->fence->seq == RADEON_FENCE_NOTEMITED_SEQ ||
+		    radeon_fence_signaled(tmp->fence)) {
+			continue;
+		}
+		/* select the oldest unsignaled fence */
+		if (sa_bo == NULL || tmp->fence->seq < sa_bo->fence->seq) {
+			sa_bo = tmp;
+		}
+	}
+	spin_unlock(&sa_manager->lock);
+
+	if (sa_bo == NULL) {
+		return;
+	}
+
+	/* init cmd buffer */
+	cmd.id = RATI_CMD_BUFFER;
+	cmd.ver = 1;
+	switch (ring) {
+	case RADEON_RING_TYPE_GFX_INDEX:
+		cmd.ring = RATI_RING_GFX;
+		break;
+	case CAYMAN_RING_TYPE_CP1_INDEX:
+	case CAYMAN_RING_TYPE_CP2_INDEX:
+		cmd.ring = RATI_RING_COMPUTE;
+		break;
+	default:
+		return;
+	}
+	cmd.ndw = (sa_bo->eoffset - sa_bo->soffset) >> 2;
+	cmd.paded_ndw = ALIGN(cmd.ndw, 2);
+
+	/* update header */
+	rdev->blob_header.ncmd_buffers = 1;
+	rdev->blob_header.ndata_buffers = sa_bo->nbuffers;
+
+	/* compute blob size */
+	rdev->blob_size = sizeof(rdev->blob_header) + sizeof(cmd);
+	rdev->blob_size += sa_bo->nbuffers * sizeof(struct rati_data_buffer);
+	rdev->blob_size += cmd.paded_ndw * 4;
+
+	for (i = 0; i < sa_bo->nbuffers; i++) {
+		rdev->blob_size += sa_bo->buffers[i].buffer.paded_ndw * 4;
+	}
+
+	rdev->blob = vmalloc(rdev->blob_size);
+	if (rdev->blob == NULL) {
+		dev_err(rdev->dev, "failed allocating %lldkb for lockup dump\n", rdev->blob_size >> 10);
+		return;
+	}
+
+	/* build blob */
+	memcpy(rdev->blob, &rdev->blob_header, sizeof(rdev->blob_header));
+	offset += sizeof(rdev->blob_header);
+	memcpy(rdev->blob + offset, &cmd, sizeof(cmd));
+	offset += sizeof(cmd);
+	memcpy(rdev->blob + offset, radeon_sa_bo_cpu_addr(sa_bo), cmd.ndw * 4);
+	offset += cmd.paded_ndw * 4;
+	for (i = 0; i < sa_bo->nbuffers; i++) {
+		void *ptr = NULL;
+		int r;
+
+		r = radeon_bo_reserve(sa_bo->buffers[i].bo, true);
+		if (!r) {
+			if (!radeon_bo_kmap(sa_bo->buffers[i].bo, &ptr)) {
+				sa_bo->buffers[i].buffer.flags |= RATI_DATA_VALID;
+			}
+		}
+
+		memcpy(rdev->blob + offset, &sa_bo->buffers[i].buffer,
+		       sizeof(struct rati_data_buffer));
+		offset += sizeof(struct rati_data_buffer);
+
+		if (ptr) {
+			memcpy(rdev->blob + offset, ptr,
+			       sa_bo->buffers[i].buffer.size);
+			radeon_bo_kunmap(sa_bo->buffers[i].bo);
+		}
+		if (!r) {
+			radeon_bo_unreserve(sa_bo->buffers[i].bo);
+		}
+		offset += sa_bo->buffers[i].buffer.paded_ndw * 4;
+	}
+
+	dev_err(rdev->dev, "added %lldkb lockup dump\n", rdev->blob_size >> 10);
+}
+
 #if defined(CONFIG_DEBUG_FS)
+ssize_t radeon_blob_read(struct file *filp, char __user *ubuf,
+			 size_t max, loff_t *ppos)
+{
+	struct seq_file *m = filp->private_data;
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct radeon_device *rdev = dev->dev_private;
+
+	return simple_read_from_buffer(ubuf, max, ppos, rdev->blob, rdev->blob_size);
+}
+
 void radeon_sa_bo_dump_debug_info(struct radeon_sa_manager *sa_manager,
 				  struct seq_file *m)
 {
diff --git a/include/drm/radeon_drm.h b/include/drm/radeon_drm.h
index 5805686..a3d351e 100644
--- a/include/drm/radeon_drm.h
+++ b/include/drm/radeon_drm.h
@@ -971,4 +971,106 @@ struct drm_radeon_info {
 	uint64_t		value;
 };
 
+
+/*
+ * RATI dump file format
+ */
+
+/* List of uniq identifiant of each structure, the identifiant of the header
+ * also serve as file signature.
+ */
+#define RATI_HEADER         0xCAFEDEAD
+#define RATI_CMD_BUFFER     1
+#define RATI_DATA_BUFFER    2
+
+/*
+ * define ring
+ */
+#define RATI_RING_UNKNOWN   0
+#define RATI_RING_GFX       1
+#define RATI_RING_COMPUTE   2
+
+/* struct rati_header
+ *
+ * Header of the file
+ *
+ * @id:             uniq identifiant of the structure must be RATI_HEADER
+ * @ver:            version
+ * @pciid:          pciid of the GPU
+ * @family:         GPU family
+ * @ndata_buffers:  number of data buffer
+ * @ncmd_buffers:   number of cmd buffer
+ */
+struct rati_header {
+	uint32_t		id;
+	uint32_t		ver;
+	uint32_t		pciid;
+	uint32_t		ndata_buffers;
+	uint32_t		ncmd_buffers;
+	uint32_t		pad;
+	/* end of version 1 */
+};
+
+/* struct rati_cmd_buffer
+ *
+ * cmd buffer, follow by paded_ndw (paded ndw must leave next struct
+ * on 64bits aligned offset)
+ *
+ * @id:             uniq identifiant of the structure must be RATI_CMD_BUFFER
+ * @ver:            version
+ * @paded_ndw:      paded number of dwords
+ * @ndw:            ndwords in this command buffer
+ * @ring:           which ring this cmd buffer should be executed on
+ */
+struct rati_cmd_buffer {
+	uint32_t		id;
+	uint32_t		ver;
+	uint32_t		paded_ndw;
+	uint32_t		ndw;
+	uint32_t		ring;
+	uint32_t		pad;
+	/* end of version 1 */
+};
+
+/* struct rati_data_buffer
+ *
+ * data buffer, follow by paded_ndw (paded ndw must leave next struct
+ * on 64bits aligned offset)
+ *
+ * @id:             uniq identifiant of the structure must be RATI_DATA_BUFFER
+ * @ver:            header version
+ * @alignment:      alignment of buffer
+ * @paded_ndw:      paded number of dwords
+ * @size:           size of buffer in byte
+ * @offset:         offset of this buffer while this was captured (could be 0
+ *                  for all buffer is capturing from userspace)
+ * @flags:          various flags that tells all the purpose for which the
+ *                  buffer is use (nothing is exclusive same buffer can be
+ *                  a texture and zbuffer or all of possible at the same
+ *                  time. Kernel will only set the valid flag bit, it's up
+ *                  to userspace tools to set more flags.
+ */
+#define	RATI_DATA_VALID			(1 << 0)
+#define	RATI_DATA_CBUF			(1 << 1)	/* color buffer */
+#define	RATI_DATA_ZBUF			(1 << 2)	/* z buffer */
+#define	RATI_DATA_SBUF			(1 << 3)	/* stencil buffer */
+#define	RATI_DATA_TBUF			(1 << 4)	/* tile buffer for color buffer */
+#define	RATI_DATA_FBUF			(1 << 5)	/* fmask buffer for color buffer */
+#define	RATI_DATA_HBUF			(1 << 6)	/* htile buffer for z/stencil buffer */
+#define	RATI_DATA_TEX			(1 << 7)	/* texture */
+#define	RATI_DATA_VS			(1 << 8)	/* vertex shader */
+#define	RATI_DATA_PS			(1 << 9)	/* pixel shader */
+
+struct rati_data_buffer {
+	uint32_t		id;
+	uint32_t		ver;
+	uint32_t		alignment;
+	uint32_t		pad;
+	uint64_t		paded_ndw;
+	uint64_t		size;
+	uint64_t		offset;
+	uint64_t		flags;
+	/* end of version 1 */
+};
+
 #endif
-- 
1.7.7.6

[PATCH 5/5] drm/radeon: restore consistant whitespace & indentation

[j.glisse]

From: Jerome Glisse <jglisse@redhat.com>

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h      |  528 +++++++++++++++++-----------------
 drivers/gpu/drm/radeon/radeon_ring.c |    3 +-
 2 files changed, 267 insertions(+), 264 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index f27fca6..f050be2 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -132,9 +132,9 @@ extern int radeon_lockup_timeout;
  * Errata workarounds.
  */
 enum radeon_pll_errata {
-	CHIP_ERRATA_R300_CG             = 0x00000001,
-	CHIP_ERRATA_PLL_DUMMYREADS      = 0x00000002,
-	CHIP_ERRATA_PLL_DELAY           = 0x00000004
+	CHIP_ERRATA_R300_CG		= 0x00000001,
+	CHIP_ERRATA_PLL_DUMMYREADS	= 0x00000002,
+	CHIP_ERRATA_PLL_DELAY		= 0x00000004
 };
 
 
@@ -218,17 +218,17 @@ void radeon_dummy_page_fini(struct radeon_device *rdev);
  * Clocks
  */
 struct radeon_clock {
-	struct radeon_pll p1pll;
-	struct radeon_pll p2pll;
-	struct radeon_pll dcpll;
-	struct radeon_pll spll;
-	struct radeon_pll mpll;
+	struct radeon_pll	p1pll;
+	struct radeon_pll	p2pll;
+	struct radeon_pll	dcpll;
+	struct radeon_pll	spll;
+	struct radeon_pll	mpll;
 	/* 10 Khz units */
-	uint32_t default_mclk;
-	uint32_t default_sclk;
-	uint32_t default_dispclk;
-	uint32_t dp_extclk;
-	uint32_t max_pixel_clock;
+	uint32_t		default_mclk;
+	uint32_t		default_sclk;
+	uint32_t		default_dispclk;
+	uint32_t		dp_extclk;
+	uint32_t		max_pixel_clock;
 };
 
 /*
@@ -296,7 +296,7 @@ unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring);
  * Tiling registers
  */
 struct radeon_surface_reg {
-	struct radeon_bo *bo;
+	struct radeon_bo		*bo;
 };
 
 #define RADEON_GEM_MAX_SURFACES 8
@@ -305,7 +305,7 @@ struct radeon_surface_reg {
  * TTM.
  */
 struct radeon_mman {
-	struct ttm_bo_global_ref        bo_global_ref;
+	struct ttm_bo_global_ref	bo_global_ref;
 	struct drm_global_reference	mem_global_ref;
 	struct ttm_bo_device		bdev;
 	bool				mem_global_referenced;
@@ -351,12 +351,12 @@ struct radeon_bo {
 #define gem_to_radeon_bo(gobj) container_of((gobj), struct radeon_bo, gem_base)
 
 struct radeon_bo_list {
-	struct ttm_validate_buffer tv;
-	struct radeon_bo	*bo;
-	uint64_t		gpu_offset;
-	unsigned		rdomain;
-	unsigned		wdomain;
-	u32			tiling_flags;
+	struct ttm_validate_buffer	tv;
+	struct radeon_bo		*bo;
+	uint64_t			gpu_offset;
+	unsigned			rdomain;
+	unsigned			wdomain;
+	u32				tiling_flags;
 };
 
 /* sub-allocation manager, it has to be protected by another lock.
@@ -521,7 +521,7 @@ struct radeon_mc {
 	int			vram_mtrr;
 	bool			vram_is_ddr;
 	bool			igp_sideport_enabled;
-	u64                     gtt_base_align;
+	u64			gtt_base_align;
 };
 
 bool radeon_combios_sideport_present(struct radeon_device *rdev);
@@ -532,7 +532,7 @@ bool radeon_atombios_sideport_present(struct radeon_device *rdev);
  */
 struct radeon_scratch {
 	unsigned		num_reg;
-	uint32_t                reg_base;
+	uint32_t		reg_base;
 	bool			free[32];
 	uint32_t		reg[32];
 };
@@ -546,55 +546,55 @@ void radeon_scratch_free(struct radeon_device *rdev, uint32_t reg);
  */
 
 struct radeon_unpin_work {
-	struct work_struct work;
-	struct radeon_device *rdev;
-	int crtc_id;
-	struct radeon_fence *fence;
-	struct drm_pending_vblank_event *event;
-	struct radeon_bo *old_rbo;
-	u64 new_crtc_base;
+	struct work_struct			work;
+	struct radeon_device			*rdev;
+	int					crtc_id;
+	struct radeon_fence			*fence;
+	struct drm_pending_vblank_event		*event;
+	struct radeon_bo			*old_rbo;
+	u64					new_crtc_base;
 };
 
 struct r500_irq_stat_regs {
-	u32 disp_int;
-	u32 hdmi0_status;
+	u32	disp_int;
+	u32	hdmi0_status;
 };
 
 struct r600_irq_stat_regs {
-	u32 disp_int;
-	u32 disp_int_cont;
-	u32 disp_int_cont2;
-	u32 d1grph_int;
-	u32 d2grph_int;
-	u32 hdmi0_status;
-	u32 hdmi1_status;
+	u32	disp_int;
+	u32	disp_int_cont;
+	u32	disp_int_cont2;
+	u32	d1grph_int;
+	u32	d2grph_int;
+	u32	hdmi0_status;
+	u32	hdmi1_status;
 };
 
 struct evergreen_irq_stat_regs {
-	u32 disp_int;
-	u32 disp_int_cont;
-	u32 disp_int_cont2;
-	u32 disp_int_cont3;
-	u32 disp_int_cont4;
-	u32 disp_int_cont5;
-	u32 d1grph_int;
-	u32 d2grph_int;
-	u32 d3grph_int;
-	u32 d4grph_int;
-	u32 d5grph_int;
-	u32 d6grph_int;
-	u32 afmt_status1;
-	u32 afmt_status2;
-	u32 afmt_status3;
-	u32 afmt_status4;
-	u32 afmt_status5;
-	u32 afmt_status6;
+	u32	disp_int;
+	u32	disp_int_cont;
+	u32	disp_int_cont2;
+	u32	disp_int_cont3;
+	u32	disp_int_cont4;
+	u32	disp_int_cont5;
+	u32	d1grph_int;
+	u32	d2grph_int;
+	u32	d3grph_int;
+	u32	d4grph_int;
+	u32	d5grph_int;
+	u32	d6grph_int;
+	u32	afmt_status1;
+	u32	afmt_status2;
+	u32	afmt_status3;
+	u32	afmt_status4;
+	u32	afmt_status5;
+	u32	afmt_status6;
 };
 
 union radeon_irq_stat_regs {
-	struct r500_irq_stat_regs r500;
-	struct r600_irq_stat_regs r600;
-	struct evergreen_irq_stat_regs evergreen;
+	struct r500_irq_stat_regs		r500;
+	struct r600_irq_stat_regs		r600;
+	struct evergreen_irq_stat_regs		evergreen;
 };
 
 #define RADEON_MAX_HPD_PINS 6
@@ -602,21 +602,21 @@ union radeon_irq_stat_regs {
 #define RADEON_MAX_AFMT_BLOCKS 6
 
 struct radeon_irq {
-	bool		installed;
-	bool		sw_int[RADEON_NUM_RINGS];
-	bool		crtc_vblank_int[RADEON_MAX_CRTCS];
-	bool		pflip[RADEON_MAX_CRTCS];
-	wait_queue_head_t	vblank_queue;
-	bool            hpd[RADEON_MAX_HPD_PINS];
-	bool            gui_idle;
-	bool            gui_idle_acked;
-	wait_queue_head_t	idle_queue;
-	bool		afmt[RADEON_MAX_AFMT_BLOCKS];
-	spinlock_t sw_lock;
-	int sw_refcount[RADEON_NUM_RINGS];
-	union radeon_irq_stat_regs stat_regs;
-	spinlock_t pflip_lock[RADEON_MAX_CRTCS];
-	int pflip_refcount[RADEON_MAX_CRTCS];
+	bool				installed;
+	bool				sw_int[RADEON_NUM_RINGS];
+	bool				crtc_vblank_int[RADEON_MAX_CRTCS];
+	bool				pflip[RADEON_MAX_CRTCS];
+	wait_queue_head_t		vblank_queue;
+	bool				hpd[RADEON_MAX_HPD_PINS];
+	bool				gui_idle;
+	bool				gui_idle_acked;
+	wait_queue_head_t		idle_queue;
+	bool				afmt[RADEON_MAX_AFMT_BLOCKS];
+	spinlock_t			sw_lock;
+	int				sw_refcount[RADEON_NUM_RINGS];
+	union radeon_irq_stat_regs	stat_regs;
+	spinlock_t			pflip_lock[RADEON_MAX_CRTCS];
+	int				pflip_refcount[RADEON_MAX_CRTCS];
 };
 
 int radeon_irq_kms_init(struct radeon_device *rdev);
@@ -729,8 +729,8 @@ struct r600_ih {
 	unsigned		ring_size;
 	uint64_t		gpu_addr;
 	uint32_t		ptr_mask;
-	spinlock_t              lock;
-	bool                    enabled;
+	spinlock_t		lock;
+	bool			enabled;
 };
 
 struct r600_blit_cp_primitives {
@@ -751,15 +751,15 @@ struct r600_blit_cp_primitives {
 };
 
 struct r600_blit {
-	struct radeon_bo	*shader_obj;
-	struct r600_blit_cp_primitives primitives;
-	int max_dim;
-	int ring_size_common;
-	int ring_size_per_loop;
-	u64 shader_gpu_addr;
-	u32 vs_offset, ps_offset;
-	u32 state_offset;
-	u32 state_len;
+	struct radeon_bo		*shader_obj;
+	struct r600_blit_cp_primitives	primitives;
+	int				max_dim;
+	int				ring_size_common;
+	int				ring_size_per_loop;
+	u64				shader_gpu_addr;
+	u32				vs_offset, ps_offset;
+	u32				state_offset;
+	u32				state_len;
 };
 
 void r600_blit_suspend(struct radeon_device *rdev);
@@ -894,8 +894,8 @@ struct radeon_wb {
 	struct radeon_bo	*wb_obj;
 	volatile uint32_t	*wb;
 	uint64_t		gpu_addr;
-	bool                    enabled;
-	bool                    use_event;
+	bool			enabled;
+	bool			use_event;
 };
 
 #define RADEON_WB_SCRATCH_OFFSET 0
@@ -995,19 +995,23 @@ enum radeon_int_thermal_type {
 };
 
 struct radeon_voltage {
-	enum radeon_voltage_type type;
+	enum radeon_voltage_type	type;
 	/* gpio voltage */
-	struct radeon_gpio_rec gpio;
-	u32 delay; /* delay in usec from voltage drop to sclk change */
-	bool active_high; /* voltage drop is active when bit is high */
+	struct radeon_gpio_rec		gpio;
+	/* delay in usec from voltage drop to sclk change */
+	u32				delay;
+	/* voltage drop is active when bit is high */
+	bool				active_high;
 	/* VDDC voltage */
-	u8 vddc_id; /* index into vddc voltage table */
-	u8 vddci_id; /* index into vddci voltage table */
-	bool vddci_enabled;
+	/* index into vddc voltage table */
+	u8				vddc_id;
+	/* index into vddci voltage table */
+	u8				vddci_id;
+	bool				vddci_enabled;
 	/* r6xx+ sw */
-	u16 voltage;
+	u16				voltage;
 	/* evergreen+ vddci */
-	u16 vddci;
+	u16				vddci;
 };
 
 /* clock mode flags */
@@ -1015,29 +1019,29 @@ struct radeon_voltage {
 
 struct radeon_pm_clock_info {
 	/* memory clock */
-	u32 mclk;
+	u32				mclk;
 	/* engine clock */
-	u32 sclk;
+	u32				sclk;
 	/* voltage info */
-	struct radeon_voltage voltage;
+	struct radeon_voltage		voltage;
 	/* standardized clock flags */
-	u32 flags;
+	u32				flags;
 };
 
 /* state flags */
 #define RADEON_PM_STATE_SINGLE_DISPLAY_ONLY (1 << 0)
 
 struct radeon_power_state {
-	enum radeon_pm_state_type type;
-	struct radeon_pm_clock_info *clock_info;
+	enum radeon_pm_state_type	type;
+	struct radeon_pm_clock_info	*clock_info;
 	/* number of valid clock modes in this power state */
-	int num_clock_modes;
-	struct radeon_pm_clock_info *default_clock_mode;
+	int				num_clock_modes;
+	struct radeon_pm_clock_info	*default_clock_mode;
 	/* standardized state flags */
-	u32 flags;
-	u32 misc; /* vbios specific flags */
-	u32 misc2; /* vbios specific flags */
-	int pcie_lanes; /* pcie lanes */
+	u32				flags;
+	u32				misc; /* vbios specific flags */
+	u32				misc2; /* vbios specific flags */
+	int				pcie_lanes; /* pcie lanes */
 };
 
 /*
@@ -1046,57 +1050,57 @@ struct radeon_power_state {
 #define RADEON_MODE_OVERCLOCK_MARGIN 500 /* 5 MHz */
 
 struct radeon_pm {
-	struct mutex		mutex;
-	u32			active_crtcs;
-	int			active_crtc_count;
-	int			req_vblank;
-	bool			vblank_sync;
-	bool			gui_idle;
-	fixed20_12		max_bandwidth;
-	fixed20_12		igp_sideport_mclk;
-	fixed20_12		igp_system_mclk;
-	fixed20_12		igp_ht_link_clk;
-	fixed20_12		igp_ht_link_width;
-	fixed20_12		k8_bandwidth;
-	fixed20_12		sideport_bandwidth;
-	fixed20_12		ht_bandwidth;
-	fixed20_12		core_bandwidth;
-	fixed20_12		sclk;
-	fixed20_12		mclk;
-	fixed20_12		needed_bandwidth;
-	struct radeon_power_state *power_state;
+	struct mutex			mutex;
+	u32				active_crtcs;
+	int				active_crtc_count;
+	int				req_vblank;
+	bool				vblank_sync;
+	bool				gui_idle;
+	fixed20_12			max_bandwidth;
+	fixed20_12			igp_sideport_mclk;
+	fixed20_12			igp_system_mclk;
+	fixed20_12			igp_ht_link_clk;
+	fixed20_12			igp_ht_link_width;
+	fixed20_12			k8_bandwidth;
+	fixed20_12			sideport_bandwidth;
+	fixed20_12			ht_bandwidth;
+	fixed20_12			core_bandwidth;
+	fixed20_12			sclk;
+	fixed20_12			mclk;
+	fixed20_12			needed_bandwidth;
+	struct radeon_power_state	*power_state;
 	/* number of valid power states */
-	int                     num_power_states;
-	int                     current_power_state_index;
-	int                     current_clock_mode_index;
-	int                     requested_power_state_index;
-	int                     requested_clock_mode_index;
-	int                     default_power_state_index;
-	u32                     current_sclk;
-	u32                     current_mclk;
-	u16                     current_vddc;
-	u16                     current_vddci;
-	u32                     default_sclk;
-	u32                     default_mclk;
-	u16                     default_vddc;
-	u16                     default_vddci;
-	struct radeon_i2c_chan *i2c_bus;
+	int				num_power_states;
+	int				current_power_state_index;
+	int				current_clock_mode_index;
+	int				requested_power_state_index;
+	int				requested_clock_mode_index;
+	int				default_power_state_index;
+	u32				current_sclk;
+	u32				current_mclk;
+	u16				current_vddc;
+	u16				current_vddci;
+	u32				default_sclk;
+	u32				default_mclk;
+	u16				default_vddc;
+	u16				default_vddci;
+	struct radeon_i2c_chan		*i2c_bus;
 	/* selected pm method */
-	enum radeon_pm_method     pm_method;
+	enum radeon_pm_method		pm_method;
 	/* dynpm power management */
-	struct delayed_work	dynpm_idle_work;
-	enum radeon_dynpm_state	dynpm_state;
+	struct delayed_work		dynpm_idle_work;
+	enum radeon_dynpm_state		dynpm_state;
 	enum radeon_dynpm_action	dynpm_planned_action;
-	unsigned long		dynpm_action_timeout;
-	bool                    dynpm_can_upclock;
-	bool                    dynpm_can_downclock;
+	unsigned long			dynpm_action_timeout;
+	bool				dynpm_can_upclock;
+	bool				dynpm_can_downclock;
 	/* profile-based power management */
-	enum radeon_pm_profile_type profile;
-	int                     profile_index;
-	struct radeon_pm_profile profiles[PM_PROFILE_MAX];
+	enum radeon_pm_profile_type	profile;
+	int				profile_index;
+	struct radeon_pm_profile	profiles[PM_PROFILE_MAX];
 	/* internal thermal controller on rv6xx+ */
-	enum radeon_int_thermal_type int_thermal_type;
-	struct device	        *int_hwmon_dev;
+	enum radeon_int_thermal_type	int_thermal_type;
+	struct device			*int_hwmon_dev;
 };
 
 int radeon_pm_get_type_index(struct radeon_device *rdev,
@@ -1319,97 +1323,97 @@ struct rv770_asic {
 };
 
 struct evergreen_asic {
-	unsigned num_ses;
-	unsigned max_pipes;
-	unsigned max_tile_pipes;
-	unsigned max_simds;
-	unsigned max_backends;
-	unsigned max_gprs;
-	unsigned max_threads;
-	unsigned max_stack_entries;
-	unsigned max_hw_contexts;
-	unsigned max_gs_threads;
-	unsigned sx_max_export_size;
-	unsigned sx_max_export_pos_size;
-	unsigned sx_max_export_smx_size;
-	unsigned sq_num_cf_insts;
-	unsigned sx_num_of_sets;
-	unsigned sc_prim_fifo_size;
-	unsigned sc_hiz_tile_fifo_size;
-	unsigned sc_earlyz_tile_fifo_size;
-	unsigned tiling_nbanks;
-	unsigned tiling_npipes;
-	unsigned tiling_group_size;
-	unsigned tile_config;
-	unsigned backend_map;
+	unsigned		num_ses;
+	unsigned		max_pipes;
+	unsigned		max_tile_pipes;
+	unsigned		max_simds;
+	unsigned		max_backends;
+	unsigned		max_gprs;
+	unsigned		max_threads;
+	unsigned		max_stack_entries;
+	unsigned		max_hw_contexts;
+	unsigned		max_gs_threads;
+	unsigned		sx_max_export_size;
+	unsigned		sx_max_export_pos_size;
+	unsigned		sx_max_export_smx_size;
+	unsigned		sq_num_cf_insts;
+	unsigned		sx_num_of_sets;
+	unsigned		sc_prim_fifo_size;
+	unsigned		sc_hiz_tile_fifo_size;
+	unsigned		sc_earlyz_tile_fifo_size;
+	unsigned		tiling_nbanks;
+	unsigned		tiling_npipes;
+	unsigned		tiling_group_size;
+	unsigned		tile_config;
+	unsigned		backend_map;
 };
 
 struct cayman_asic {
-	unsigned max_shader_engines;
-	unsigned max_pipes_per_simd;
-	unsigned max_tile_pipes;
-	unsigned max_simds_per_se;
-	unsigned max_backends_per_se;
-	unsigned max_texture_channel_caches;
-	unsigned max_gprs;
-	unsigned max_threads;
-	unsigned max_gs_threads;
-	unsigned max_stack_entries;
-	unsigned sx_num_of_sets;
-	unsigned sx_max_export_size;
-	unsigned sx_max_export_pos_size;
-	unsigned sx_max_export_smx_size;
-	unsigned max_hw_contexts;
-	unsigned sq_num_cf_insts;
-	unsigned sc_prim_fifo_size;
-	unsigned sc_hiz_tile_fifo_size;
-	unsigned sc_earlyz_tile_fifo_size;
-
-	unsigned num_shader_engines;
-	unsigned num_shader_pipes_per_simd;
-	unsigned num_tile_pipes;
-	unsigned num_simds_per_se;
-	unsigned num_backends_per_se;
-	unsigned backend_disable_mask_per_asic;
-	unsigned backend_map;
-	unsigned num_texture_channel_caches;
-	unsigned mem_max_burst_length_bytes;
-	unsigned mem_row_size_in_kb;
-	unsigned shader_engine_tile_size;
-	unsigned num_gpus;
-	unsigned multi_gpu_tile_size;
-
-	unsigned tile_config;
+	unsigned		max_shader_engines;
+	unsigned		max_pipes_per_simd;
+	unsigned		max_tile_pipes;
+	unsigned		max_simds_per_se;
+	unsigned		max_backends_per_se;
+	unsigned		max_texture_channel_caches;
+	unsigned		max_gprs;
+	unsigned		max_threads;
+	unsigned		max_gs_threads;
+	unsigned		max_stack_entries;
+	unsigned		sx_num_of_sets;
+	unsigned		sx_max_export_size;
+	unsigned		sx_max_export_pos_size;
+	unsigned		sx_max_export_smx_size;
+	unsigned		max_hw_contexts;
+	unsigned		sq_num_cf_insts;
+	unsigned		sc_prim_fifo_size;
+	unsigned		sc_hiz_tile_fifo_size;
+	unsigned		sc_earlyz_tile_fifo_size;
+
+	unsigned		num_shader_engines;
+	unsigned		num_shader_pipes_per_simd;
+	unsigned		num_tile_pipes;
+	unsigned		num_simds_per_se;
+	unsigned		num_backends_per_se;
+	unsigned		backend_disable_mask_per_asic;
+	unsigned		backend_map;
+	unsigned		num_texture_channel_caches;
+	unsigned		mem_max_burst_length_bytes;
+	unsigned		mem_row_size_in_kb;
+	unsigned		shader_engine_tile_size;
+	unsigned		num_gpus;
+	unsigned		multi_gpu_tile_size;
+
+	unsigned		tile_config;
 };
 
 struct si_asic {
-	unsigned max_shader_engines;
-	unsigned max_pipes_per_simd;
-	unsigned max_tile_pipes;
-	unsigned max_simds_per_se;
-	unsigned max_backends_per_se;
-	unsigned max_texture_channel_caches;
-	unsigned max_gprs;
-	unsigned max_gs_threads;
-	unsigned max_hw_contexts;
-	unsigned sc_prim_fifo_size_frontend;
-	unsigned sc_prim_fifo_size_backend;
-	unsigned sc_hiz_tile_fifo_size;
-	unsigned sc_earlyz_tile_fifo_size;
-
-	unsigned num_shader_engines;
-	unsigned num_tile_pipes;
-	unsigned num_backends_per_se;
-	unsigned backend_disable_mask_per_asic;
-	unsigned backend_map;
-	unsigned num_texture_channel_caches;
-	unsigned mem_max_burst_length_bytes;
-	unsigned mem_row_size_in_kb;
-	unsigned shader_engine_tile_size;
-	unsigned num_gpus;
-	unsigned multi_gpu_tile_size;
-
-	unsigned tile_config;
+	unsigned		max_shader_engines;
+	unsigned		max_pipes_per_simd;
+	unsigned		max_tile_pipes;
+	unsigned		max_simds_per_se;
+	unsigned		max_backends_per_se;
+	unsigned		max_texture_channel_caches;
+	unsigned		max_gprs;
+	unsigned		max_gs_threads;
+	unsigned		max_hw_contexts;
+	unsigned		sc_prim_fifo_size_frontend;
+	unsigned		sc_prim_fifo_size_backend;
+	unsigned		sc_hiz_tile_fifo_size;
+	unsigned		sc_earlyz_tile_fifo_size;
+
+	unsigned		num_shader_engines;
+	unsigned		num_tile_pipes;
+	unsigned		num_backends_per_se;
+	unsigned		backend_disable_mask_per_asic;
+	unsigned		backend_map;
+	unsigned		num_texture_channel_caches;
+	unsigned		mem_max_burst_length_bytes;
+	unsigned		mem_row_size_in_kb;
+	unsigned		shader_engine_tile_size;
+	unsigned		num_gpus;
+	unsigned		multi_gpu_tile_size;
+
+	unsigned		tile_config;
 };
 
 union radeon_asic_config {
@@ -1530,32 +1534,32 @@ struct radeon_device {
 	bool				suspend;
 	bool				need_dma32;
 	bool				accel_working;
-	struct radeon_surface_reg surface_regs[RADEON_GEM_MAX_SURFACES];
-	const struct firmware *me_fw;	/* all family ME firmware */
-	const struct firmware *pfp_fw;	/* r6/700 PFP firmware */
-	const struct firmware *rlc_fw;	/* r6/700 RLC firmware */
-	const struct firmware *mc_fw;	/* NI MC firmware */
-	const struct firmware *ce_fw;	/* SI CE firmware */
-	struct r600_blit r600_blit;
-	struct r600_vram_scratch vram_scratch;
-	int msi_enabled; /* msi enabled */
-	struct r600_ih ih; /* r6/700 interrupt ring */
-	struct si_rlc rlc;
-	struct work_struct hotplug_work;
-	struct work_struct audio_work;
-	int num_crtc; /* number of crtcs */
-	struct mutex dc_hw_i2c_mutex; /* display controller hw i2c mutex */
-	struct mutex vram_mutex;
-	struct r600_audio audio; /* audio stuff */
-	struct notifier_block acpi_nb;
+	struct radeon_surface_reg	surface_regs[RADEON_GEM_MAX_SURFACES];
+	const struct firmware		*me_fw;	/* all family ME firmware */
+	const struct firmware		*pfp_fw;	/* r6/700 PFP firmware */
+	const struct firmware		*rlc_fw;	/* r6/700 RLC firmware */
+	const struct firmware		*mc_fw;	/* NI MC firmware */
+	const struct firmware		*ce_fw;	/* SI CE firmware */
+	struct r600_blit		r600_blit;
+	struct r600_vram_scratch	vram_scratch;
+	int				msi_enabled; /* msi enabled */
+	struct r600_ih			ih; /* r6/700 interrupt ring */
+	struct si_rlc 			rlc;
+	struct work_struct		hotplug_work;
+	struct work_struct		audio_work;
+	int				num_crtc; /* number of crtcs */
+	struct mutex			dc_hw_i2c_mutex; /* display controller hw i2c mutex */
+	struct mutex			vram_mutex;
+	struct r600_audio		audio; /* audio stuff */
+	struct notifier_block		acpi_nb;
 	/* only one userspace can use Hyperz features or CMASK at a time */
-	struct drm_file *hyperz_filp;
-	struct drm_file *cmask_filp;
+	struct drm_file			*hyperz_filp;
+	struct drm_file			*cmask_filp;
 	/* i2c buses */
-	struct radeon_i2c_chan *i2c_bus[RADEON_MAX_I2C_BUS];
+	struct radeon_i2c_chan		*i2c_bus[RADEON_MAX_I2C_BUS];
 	/* debugfs */
-	struct radeon_debugfs	debugfs[RADEON_DEBUGFS_MAX_COMPONENTS];
-	unsigned 		debugfs_count;
+	struct radeon_debugfs		debugfs[RADEON_DEBUGFS_MAX_COMPONENTS];
+	unsigned 			debugfs_count;
 	/* virtual memory */
 	struct radeon_vm_manager	vm_manager;
 	/* lockup blob dumping */
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 6982d8f..9f8d9d7 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -494,7 +494,6 @@ static int radeon_debugfs_sa_info(struct seq_file *m, void *data)
 	radeon_sa_bo_dump_debug_info(&rdev->ring_tmp_bo, m);
 
 	return 0;
-
 }
 
 static int radeon_debugfs_lockup(struct seq_file *m, void *data)
@@ -504,7 +503,7 @@ static int radeon_debugfs_lockup(struct seq_file *m, void *data)
 }
 
 static struct drm_info_list radeon_debugfs_sa_list[] = {
-        {"radeon_sa_info", &radeon_debugfs_sa_info, 0, NULL},
+	{"radeon_sa_info", &radeon_debugfs_sa_info, 0, NULL},
 };
 
 static struct drm_info_list radeon_debugfs_lockup_list[] = {
-- 
1.7.7.6

Re: GPU lockup dumping

[Dave Airlie]

On Thu, May 17, 2012 at 7:28 PM,  <j.glisse@gmail.com> wrote:
> So here is improved patchset, where i splited ground work necessary
> for the dumping into their own patch. The debugfs improvement could
> probably be usefull to intel instead of having i915 have it's own
> debugfs file stuff.
>
> The lockup dumping public api have been move into radeon_drm.h
>
> Stressing the fact again that dump are self contained ie they have
> all the data needed to be replayed (vertex, indices, shader, texture,
> ...).
>
> Would really like to get this into 3.5, the new API is pretty much
> straightforward and userspace tools can easily be made to convert
> it to other format. The change to the driver is self contained.

I really don't like introducing this at this stage into 3.5,

I'd really like a good review of the API and what information we provide
along with how extensible it is.

I'm still not convinced replay is what we want in the field, I know its what
*you* want, but I think apitrace stuff in userspace pretty much covers
the replaying situation. So I'd have to look at this and see how easy
it makes disecting command streams etc.

Dave.

Re: GPU lockup dumping

[Christian König]

On 23.05.2012 11:27, Dave Airlie wrote:
> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>  wrote:
>> So here is improved patchset, where i splited ground work necessary
>> for the dumping into their own patch. The debugfs improvement could
>> probably be usefull to intel instead of having i915 have it's own
>> debugfs file stuff.
>>
>> The lockup dumping public api have been move into radeon_drm.h
>>
>> Stressing the fact again that dump are self contained ie they have
>> all the data needed to be replayed (vertex, indices, shader, texture,
>> ...).
>>
>> Would really like to get this into 3.5, the new API is pretty much
>> straightforward and userspace tools can easily be made to convert
>> it to other format. The change to the driver is self contained.
> I really don't like introducing this at this stage into 3.5,
>
> I'd really like a good review of the API and what information we provide
> along with how extensible it is.
>
> I'm still not convinced replay is what we want in the field, I know its what
> *you* want, but I think apitrace stuff in userspace pretty much covers
> the replaying situation. So I'd have to look at this and see how easy
> it makes disecting command streams etc.
>
> Dave.

I agree that it might not be a good idea to push that into 3.5, since at 
least I (and I also think Alex) didn't had time to look into it yet. On 
the other hand the patches look quite reasonable.

But I still wanted to throw in a requirement from my day to day work, 
maybe that helps finding a more general solution:
When we start to work with more parts of the chip it might be necessary 
to dump everything that is currently "in the fly". For example I had a 
whole bunch of problems where copying data around with a 3D Blit and 
then missing a sync between this job and a job on another rings causes a 
"hiccup" in the hardware.

I know that this isn't your focus and that is absolutely ok with me, 
cause the format you are introducing is just used in debugfs and so not 
part of any stable API (at least not in my understanding), but you 
should still keep in mind that we might need to extend it into that 
direction in the future.

Christian.

Re: GPU lockup dumping

[Jerome Glisse]

On Wed, May 23, 2012 at 8:34 AM, Christian König
<deathsimple@vodafone.de> wrote:
> On 23.05.2012 11:27, Dave Airlie wrote:
>>
>> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>  wrote:
>>>
>>> So here is improved patchset, where i splited ground work necessary
>>> for the dumping into their own patch. The debugfs improvement could
>>> probably be usefull to intel instead of having i915 have it's own
>>> debugfs file stuff.
>>>
>>> The lockup dumping public api have been move into radeon_drm.h
>>>
>>> Stressing the fact again that dump are self contained ie they have
>>> all the data needed to be replayed (vertex, indices, shader, texture,
>>> ...).
>>>
>>> Would really like to get this into 3.5, the new API is pretty much
>>> straightforward and userspace tools can easily be made to convert
>>> it to other format. The change to the driver is self contained.
>>
>> I really don't like introducing this at this stage into 3.5,
>>
>> I'd really like a good review of the API and what information we provide
>> along with how extensible it is.
>>
>> I'm still not convinced replay is what we want in the field, I know its
>> what
>> *you* want, but I think apitrace stuff in userspace pretty much covers
>> the replaying situation. So I'd have to look at this and see how easy
>> it makes disecting command streams etc.
>>
>> Dave.
>
>
> I agree that it might not be a good idea to push that into 3.5, since at
> least I (and I also think Alex) didn't had time to look into it yet. On the
> other hand the patches look quite reasonable.
>
> But I still wanted to throw in a requirement from my day to day work, maybe
> that helps finding a more general solution:
> When we start to work with more parts of the chip it might be necessary to
> dump everything that is currently "in the fly". For example I had a whole
> bunch of problems where copying data around with a 3D Blit and then missing
> a sync between this job and a job on another rings causes a "hiccup" in the
> hardware.
>
> I know that this isn't your focus and that is absolutely ok with me, cause
> the format you are introducing is just used in debugfs and so not part of
> any stable API (at least not in my understanding), but you should still keep
> in mind that we might need to extend it into that direction in the future.
>
> Christian.

Note that my format is also done with that in mind, it can capture ib
from all rings. The only thing i don't think worth capturing are the
ring themself because there would be no way to replay them without
adding some new special API.

Cheers,
Jerome

Re: GPU lockup dumping

[Dave Airlie]

On Wed, May 23, 2012 at 3:48 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Wed, May 23, 2012 at 8:34 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> On 23.05.2012 11:27, Dave Airlie wrote:
>>>
>>> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>  wrote:
>>>>
>>>> So here is improved patchset, where i splited ground work necessary
>>>> for the dumping into their own patch. The debugfs improvement could
>>>> probably be usefull to intel instead of having i915 have it's own
>>>> debugfs file stuff.
>>>>
>>>> The lockup dumping public api have been move into radeon_drm.h
>>>>
>>>> Stressing the fact again that dump are self contained ie they have
>>>> all the data needed to be replayed (vertex, indices, shader, texture,
>>>> ...).
>>>>
>>>> Would really like to get this into 3.5, the new API is pretty much
>>>> straightforward and userspace tools can easily be made to convert
>>>> it to other format. The change to the driver is self contained.
>>>
>>> I really don't like introducing this at this stage into 3.5,
>>>
>>> I'd really like a good review of the API and what information we provide
>>> along with how extensible it is.
>>>
>>> I'm still not convinced replay is what we want in the field, I know its
>>> what
>>> *you* want, but I think apitrace stuff in userspace pretty much covers
>>> the replaying situation. So I'd have to look at this and see how easy
>>> it makes disecting command streams etc.
>>>
>>> Dave.
>>
>>
>> I agree that it might not be a good idea to push that into 3.5, since at
>> least I (and I also think Alex) didn't had time to look into it yet. On the
>> other hand the patches look quite reasonable.
>>
>> But I still wanted to throw in a requirement from my day to day work, maybe
>> that helps finding a more general solution:
>> When we start to work with more parts of the chip it might be necessary to
>> dump everything that is currently "in the fly". For example I had a whole
>> bunch of problems where copying data around with a 3D Blit and then missing
>> a sync between this job and a job on another rings causes a "hiccup" in the
>> hardware.
>>
>> I know that this isn't your focus and that is absolutely ok with me, cause
>> the format you are introducing is just used in debugfs and so not part of
>> any stable API (at least not in my understanding), but you should still keep
>> in mind that we might need to extend it into that direction in the future.
>>
>> Christian.
>
> Note that my format is also done with that in mind, it can capture ib
> from all rings. The only thing i don't think worth capturing are the
> ring themself because there would be no way to replay them without
> adding some new special API.

I'd like to dump the rings as well, as I said I'd rather we didn't
limit this to replay, but make it useful for getting as much info as
possible out

Dave.

Re: GPU lockup dumping

[Jerome Glisse]

On Wed, May 23, 2012 at 12:08 PM, Dave Airlie <airlied@gmail.com> wrote:
> On Wed, May 23, 2012 at 3:48 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
>> On Wed, May 23, 2012 at 8:34 AM, Christian König
>> <deathsimple@vodafone.de> wrote:
>>> On 23.05.2012 11:27, Dave Airlie wrote:
>>>>
>>>> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>  wrote:
>>>>>
>>>>> So here is improved patchset, where i splited ground work necessary
>>>>> for the dumping into their own patch. The debugfs improvement could
>>>>> probably be usefull to intel instead of having i915 have it's own
>>>>> debugfs file stuff.
>>>>>
>>>>> The lockup dumping public api have been move into radeon_drm.h
>>>>>
>>>>> Stressing the fact again that dump are self contained ie they have
>>>>> all the data needed to be replayed (vertex, indices, shader, texture,
>>>>> ...).
>>>>>
>>>>> Would really like to get this into 3.5, the new API is pretty much
>>>>> straightforward and userspace tools can easily be made to convert
>>>>> it to other format. The change to the driver is self contained.
>>>>
>>>> I really don't like introducing this at this stage into 3.5,
>>>>
>>>> I'd really like a good review of the API and what information we provide
>>>> along with how extensible it is.
>>>>
>>>> I'm still not convinced replay is what we want in the field, I know its
>>>> what
>>>> *you* want, but I think apitrace stuff in userspace pretty much covers
>>>> the replaying situation. So I'd have to look at this and see how easy
>>>> it makes disecting command streams etc.
>>>>
>>>> Dave.
>>>
>>>
>>> I agree that it might not be a good idea to push that into 3.5, since at
>>> least I (and I also think Alex) didn't had time to look into it yet. On the
>>> other hand the patches look quite reasonable.
>>>
>>> But I still wanted to throw in a requirement from my day to day work, maybe
>>> that helps finding a more general solution:
>>> When we start to work with more parts of the chip it might be necessary to
>>> dump everything that is currently "in the fly". For example I had a whole
>>> bunch of problems where copying data around with a 3D Blit and then missing
>>> a sync between this job and a job on another rings causes a "hiccup" in the
>>> hardware.
>>>
>>> I know that this isn't your focus and that is absolutely ok with me, cause
>>> the format you are introducing is just used in debugfs and so not part of
>>> any stable API (at least not in my understanding), but you should still keep
>>> in mind that we might need to extend it into that direction in the future.
>>>
>>> Christian.
>>
>> Note that my format is also done with that in mind, it can capture ib
>> from all rings. The only thing i don't think worth capturing are the
>> ring themself because there would be no way to replay them without
>> adding some new special API.
>
> I'd like to dump the rings as well, as I said I'd rather we didn't
> limit this to replay, but make it useful for getting as much info as
> possible out
>
> Dave.

Ring will contains very little, like ib schedule and fence, i don't
see how useful this can be.

Cheers,
Jerome

Re: GPU lockup dumping

[Dave Airlie]

On Wed, May 23, 2012 at 5:26 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Wed, May 23, 2012 at 12:08 PM, Dave Airlie <airlied@gmail.com> wrote:
>> On Wed, May 23, 2012 at 3:48 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
>>> On Wed, May 23, 2012 at 8:34 AM, Christian König
>>> <deathsimple@vodafone.de> wrote:
>>>> On 23.05.2012 11:27, Dave Airlie wrote:
>>>>>
>>>>> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>  wrote:
>>>>>>
>>>>>> So here is improved patchset, where i splited ground work necessary
>>>>>> for the dumping into their own patch. The debugfs improvement could
>>>>>> probably be usefull to intel instead of having i915 have it's own
>>>>>> debugfs file stuff.
>>>>>>
>>>>>> The lockup dumping public api have been move into radeon_drm.h
>>>>>>
>>>>>> Stressing the fact again that dump are self contained ie they have
>>>>>> all the data needed to be replayed (vertex, indices, shader, texture,
>>>>>> ...).
>>>>>>
>>>>>> Would really like to get this into 3.5, the new API is pretty much
>>>>>> straightforward and userspace tools can easily be made to convert
>>>>>> it to other format. The change to the driver is self contained.
>>>>>
>>>>> I really don't like introducing this at this stage into 3.5,
>>>>>
>>>>> I'd really like a good review of the API and what information we provide
>>>>> along with how extensible it is.
>>>>>
>>>>> I'm still not convinced replay is what we want in the field, I know its
>>>>> what
>>>>> *you* want, but I think apitrace stuff in userspace pretty much covers
>>>>> the replaying situation. So I'd have to look at this and see how easy
>>>>> it makes disecting command streams etc.
>>>>>
>>>>> Dave.
>>>>
>>>>
>>>> I agree that it might not be a good idea to push that into 3.5, since at
>>>> least I (and I also think Alex) didn't had time to look into it yet. On the
>>>> other hand the patches look quite reasonable.
>>>>
>>>> But I still wanted to throw in a requirement from my day to day work, maybe
>>>> that helps finding a more general solution:
>>>> When we start to work with more parts of the chip it might be necessary to
>>>> dump everything that is currently "in the fly". For example I had a whole
>>>> bunch of problems where copying data around with a 3D Blit and then missing
>>>> a sync between this job and a job on another rings causes a "hiccup" in the
>>>> hardware.
>>>>
>>>> I know that this isn't your focus and that is absolutely ok with me, cause
>>>> the format you are introducing is just used in debugfs and so not part of
>>>> any stable API (at least not in my understanding), but you should still keep
>>>> in mind that we might need to extend it into that direction in the future.
>>>>
>>>> Christian.
>>>
>>> Note that my format is also done with that in mind, it can capture ib
>>> from all rings. The only thing i don't think worth capturing are the
>>> ring themself because there would be no way to replay them without
>>> adding some new special API.
>>
>> I'd like to dump the rings as well, as I said I'd rather we didn't
>> limit this to replay, but make it useful for getting as much info as
>> possible out
>>
>> Dave.
>
> Ring will contains very little, like ib schedule and fence, i don't
> see how useful this can be.
>

In case we have a bug in our ib scheduling or fencing :-0

Dave.

Re: GPU lockup dumping

[Jerome Glisse]

On Wed, May 23, 2012 at 12:41 PM, Dave Airlie <airlied@gmail.com> wrote:
> On Wed, May 23, 2012 at 5:26 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
>> On Wed, May 23, 2012 at 12:08 PM, Dave Airlie <airlied@gmail.com> wrote:
>>> On Wed, May 23, 2012 at 3:48 PM, Jerome Glisse <j.glisse@gmail.com> wrote:
>>>> On Wed, May 23, 2012 at 8:34 AM, Christian König
>>>> <deathsimple@vodafone.de> wrote:
>>>>> On 23.05.2012 11:27, Dave Airlie wrote:
>>>>>>
>>>>>> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>  wrote:
>>>>>>>
>>>>>>> So here is improved patchset, where i splited ground work necessary
>>>>>>> for the dumping into their own patch. The debugfs improvement could
>>>>>>> probably be usefull to intel instead of having i915 have it's own
>>>>>>> debugfs file stuff.
>>>>>>>
>>>>>>> The lockup dumping public api have been move into radeon_drm.h
>>>>>>>
>>>>>>> Stressing the fact again that dump are self contained ie they have
>>>>>>> all the data needed to be replayed (vertex, indices, shader, texture,
>>>>>>> ...).
>>>>>>>
>>>>>>> Would really like to get this into 3.5, the new API is pretty much
>>>>>>> straightforward and userspace tools can easily be made to convert
>>>>>>> it to other format. The change to the driver is self contained.
>>>>>>
>>>>>> I really don't like introducing this at this stage into 3.5,
>>>>>>
>>>>>> I'd really like a good review of the API and what information we provide
>>>>>> along with how extensible it is.
>>>>>>
>>>>>> I'm still not convinced replay is what we want in the field, I know its
>>>>>> what
>>>>>> *you* want, but I think apitrace stuff in userspace pretty much covers
>>>>>> the replaying situation. So I'd have to look at this and see how easy
>>>>>> it makes disecting command streams etc.
>>>>>>
>>>>>> Dave.
>>>>>
>>>>>
>>>>> I agree that it might not be a good idea to push that into 3.5, since at
>>>>> least I (and I also think Alex) didn't had time to look into it yet. On the
>>>>> other hand the patches look quite reasonable.
>>>>>
>>>>> But I still wanted to throw in a requirement from my day to day work, maybe
>>>>> that helps finding a more general solution:
>>>>> When we start to work with more parts of the chip it might be necessary to
>>>>> dump everything that is currently "in the fly". For example I had a whole
>>>>> bunch of problems where copying data around with a 3D Blit and then missing
>>>>> a sync between this job and a job on another rings causes a "hiccup" in the
>>>>> hardware.
>>>>>
>>>>> I know that this isn't your focus and that is absolutely ok with me, cause
>>>>> the format you are introducing is just used in debugfs and so not part of
>>>>> any stable API (at least not in my understanding), but you should still keep
>>>>> in mind that we might need to extend it into that direction in the future.
>>>>>
>>>>> Christian.
>>>>
>>>> Note that my format is also done with that in mind, it can capture ib
>>>> from all rings. The only thing i don't think worth capturing are the
>>>> ring themself because there would be no way to replay them without
>>>> adding some new special API.
>>>
>>> I'd like to dump the rings as well, as I said I'd rather we didn't
>>> limit this to replay, but make it useful for getting as much info as
>>> possible out
>>>
>>> Dave.
>>
>> Ring will contains very little, like ib schedule and fence, i don't
>> see how useful this can be.
>>
>
> In case we have a bug in our ib scheduling or fencing :-0
>
> Dave.

Well i think we have several kind of lockup, the most basic one is
userspace sending broken shader, vertex, or something in that line.
The more complex one is timing related, like a bo move or some cache
invalidation that didn't happen properly and GPU endup reading either
wrong data or old cached data. I don't see how to capture useful
information for this second case, beside doing snapshot of memory.

For multi-ring i agree that dumping the ring might prove useful spot
inter-ring semaphore deadlock, or possibly inter-ring absence of
synchronization (but that would be a bad kernel bug).

Cheers,
Jerome

Re: GPU lockup dumping

[Christian König]

On 23.05.2012 19:02, Jerome Glisse wrote:
> On Wed, May 23, 2012 at 12:41 PM, Dave Airlie<airlied@gmail.com>  wrote:
>> On Wed, May 23, 2012 at 5:26 PM, Jerome Glisse<j.glisse@gmail.com>  wrote:
>>> On Wed, May 23, 2012 at 12:08 PM, Dave Airlie<airlied@gmail.com>  wrote:
>>>> On Wed, May 23, 2012 at 3:48 PM, Jerome Glisse<j.glisse@gmail.com>  wrote:
>>>>> On Wed, May 23, 2012 at 8:34 AM, Christian König
>>>>> <deathsimple@vodafone.de>  wrote:
>>>>>> On 23.05.2012 11:27, Dave Airlie wrote:
>>>>>>> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>    wrote:
>>>>>>>> So here is improved patchset, where i splited ground work necessary
>>>>>>>> for the dumping into their own patch. The debugfs improvement could
>>>>>>>> probably be usefull to intel instead of having i915 have it's own
>>>>>>>> debugfs file stuff.
>>>>>>>>
>>>>>>>> The lockup dumping public api have been move into radeon_drm.h
>>>>>>>>
>>>>>>>> Stressing the fact again that dump are self contained ie they have
>>>>>>>> all the data needed to be replayed (vertex, indices, shader, texture,
>>>>>>>> ...).
>>>>>>>>
>>>>>>>> Would really like to get this into 3.5, the new API is pretty much
>>>>>>>> straightforward and userspace tools can easily be made to convert
>>>>>>>> it to other format. The change to the driver is self contained.
>>>>>>> I really don't like introducing this at this stage into 3.5,
>>>>>>>
>>>>>>> I'd really like a good review of the API and what information we provide
>>>>>>> along with how extensible it is.
>>>>>>>
>>>>>>> I'm still not convinced replay is what we want in the field, I know its
>>>>>>> what
>>>>>>> *you* want, but I think apitrace stuff in userspace pretty much covers
>>>>>>> the replaying situation. So I'd have to look at this and see how easy
>>>>>>> it makes disecting command streams etc.
>>>>>>>
>>>>>>> Dave.
>>>>>>
>>>>>> I agree that it might not be a good idea to push that into 3.5, since at
>>>>>> least I (and I also think Alex) didn't had time to look into it yet. On the
>>>>>> other hand the patches look quite reasonable.
>>>>>>
>>>>>> But I still wanted to throw in a requirement from my day to day work, maybe
>>>>>> that helps finding a more general solution:
>>>>>> When we start to work with more parts of the chip it might be necessary to
>>>>>> dump everything that is currently "in the fly". For example I had a whole
>>>>>> bunch of problems where copying data around with a 3D Blit and then missing
>>>>>> a sync between this job and a job on another rings causes a "hiccup" in the
>>>>>> hardware.
>>>>>>
>>>>>> I know that this isn't your focus and that is absolutely ok with me, cause
>>>>>> the format you are introducing is just used in debugfs and so not part of
>>>>>> any stable API (at least not in my understanding), but you should still keep
>>>>>> in mind that we might need to extend it into that direction in the future.
>>>>>>
>>>>>> Christian.
>>>>> Note that my format is also done with that in mind, it can capture ib
>>>>> from all rings. The only thing i don't think worth capturing are the
>>>>> ring themself because there would be no way to replay them without
>>>>> adding some new special API.
>>>> I'd like to dump the rings as well, as I said I'd rather we didn't
>>>> limit this to replay, but make it useful for getting as much info as
>>>> possible out
>>>>
>>>> Dave.
>>> Ring will contains very little, like ib schedule and fence, i don't
>>> see how useful this can be.
>>>
>> In case we have a bug in our ib scheduling or fencing :-0
>>
>> Dave.
> Well i think we have several kind of lockup, the most basic one is
> userspace sending broken shader, vertex, or something in that line.
> The more complex one is timing related, like a bo move or some cache
> invalidation that didn't happen properly and GPU endup reading either
> wrong data or old cached data. I don't see how to capture useful
> information for this second case, beside doing snapshot of memory.
>
> For multi-ring i agree that dumping the ring might prove useful spot
> inter-ring semaphore deadlock, or possibly inter-ring absence of
> synchronization (but that would be a bad kernel bug).

I don't think that we need the actual data from the rings neither (at 
least as long as we keep the radeon_ring_* debugfs files). But it would 
still be nice to know weather or not there was a sync between the rings. 
See the patches I just send to you (sorry, actually send more patches 
than I wanted to send), storing the new sync_seq array within the debug 
output should enable us to actually figure out the dependencies and 
order between different IBs.

Cheers,
Christian.

Re: GPU lockup dumping

[Alex Deucher]

On Wed, May 23, 2012 at 8:34 AM, Christian König
<deathsimple@vodafone.de> wrote:
> On 23.05.2012 11:27, Dave Airlie wrote:
>>
>> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>  wrote:
>>>
>>> So here is improved patchset, where i splited ground work necessary
>>> for the dumping into their own patch. The debugfs improvement could
>>> probably be usefull to intel instead of having i915 have it's own
>>> debugfs file stuff.
>>>
>>> The lockup dumping public api have been move into radeon_drm.h
>>>
>>> Stressing the fact again that dump are self contained ie they have
>>> all the data needed to be replayed (vertex, indices, shader, texture,
>>> ...).
>>>
>>> Would really like to get this into 3.5, the new API is pretty much
>>> straightforward and userspace tools can easily be made to convert
>>> it to other format. The change to the driver is self contained.
>>
>> I really don't like introducing this at this stage into 3.5,
>>
>> I'd really like a good review of the API and what information we provide
>> along with how extensible it is.
>>
>> I'm still not convinced replay is what we want in the field, I know its
>> what
>> *you* want, but I think apitrace stuff in userspace pretty much covers
>> the replaying situation. So I'd have to look at this and see how easy
>> it makes disecting command streams etc.
>>
>> Dave.
>
>
> I agree that it might not be a good idea to push that into 3.5, since at
> least I (and I also think Alex) didn't had time to look into it yet. On the
> other hand the patches look quite reasonable.
>
> But I still wanted to throw in a requirement from my day to day work, maybe
> that helps finding a more general solution:
> When we start to work with more parts of the chip it might be necessary to
> dump everything that is currently "in the fly". For example I had a whole
> bunch of problems where copying data around with a 3D Blit and then missing
> a sync between this job and a job on another rings causes a "hiccup" in the
> hardware.
>
> I know that this isn't your focus and that is absolutely ok with me, cause
> the format you are introducing is just used in debugfs and so not part of
> any stable API (at least not in my understanding), but you should still keep
> in mind that we might need to extend it into that direction in the future.
>

I'm ok with it as long as we have a path to implement support for the
internal dump format so I can have the hw guys play them back on the
simulators and such.

Alex

> Christian.
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: GPU lockup dumping

[Jerome Glisse]

On Wed, May 23, 2012 at 12:04 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Wed, May 23, 2012 at 8:34 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> On 23.05.2012 11:27, Dave Airlie wrote:
>>>
>>> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>  wrote:
>>>>
>>>> So here is improved patchset, where i splited ground work necessary
>>>> for the dumping into their own patch. The debugfs improvement could
>>>> probably be usefull to intel instead of having i915 have it's own
>>>> debugfs file stuff.
>>>>
>>>> The lockup dumping public api have been move into radeon_drm.h
>>>>
>>>> Stressing the fact again that dump are self contained ie they have
>>>> all the data needed to be replayed (vertex, indices, shader, texture,
>>>> ...).
>>>>
>>>> Would really like to get this into 3.5, the new API is pretty much
>>>> straightforward and userspace tools can easily be made to convert
>>>> it to other format. The change to the driver is self contained.
>>>
>>> I really don't like introducing this at this stage into 3.5,
>>>
>>> I'd really like a good review of the API and what information we provide
>>> along with how extensible it is.
>>>
>>> I'm still not convinced replay is what we want in the field, I know its
>>> what
>>> *you* want, but I think apitrace stuff in userspace pretty much covers
>>> the replaying situation. So I'd have to look at this and see how easy
>>> it makes disecting command streams etc.
>>>
>>> Dave.
>>
>>
>> I agree that it might not be a good idea to push that into 3.5, since at
>> least I (and I also think Alex) didn't had time to look into it yet. On the
>> other hand the patches look quite reasonable.
>>
>> But I still wanted to throw in a requirement from my day to day work, maybe
>> that helps finding a more general solution:
>> When we start to work with more parts of the chip it might be necessary to
>> dump everything that is currently "in the fly". For example I had a whole
>> bunch of problems where copying data around with a 3D Blit and then missing
>> a sync between this job and a job on another rings causes a "hiccup" in the
>> hardware.
>>
>> I know that this isn't your focus and that is absolutely ok with me, cause
>> the format you are introducing is just used in debugfs and so not part of
>> any stable API (at least not in my understanding), but you should still keep
>> in mind that we might need to extend it into that direction in the future.
>>
>
> I'm ok with it as long as we have a path to implement support for the
> internal dump format so I can have the hw guys play them back on the
> simulators and such.
>
> Alex

>From what i see of the internal dump format, it's way more complex and
i don't think kernel is the right place for it, for instance you need
to set the primary surface thing, i don't want to parse cmd stream in
kernel to figure that out. I would rather have userspace tool that
postprocess thing and convert it to proper AMD dump format.

Cheers,
Jerome

Re: GPU lockup dumping

[Jerome Glisse]

On Wed, May 23, 2012 at 5:27 AM, Dave Airlie <airlied@gmail.com> wrote:
> On Thu, May 17, 2012 at 7:28 PM,  <j.glisse@gmail.com> wrote:
>> So here is improved patchset, where i splited ground work necessary
>> for the dumping into their own patch. The debugfs improvement could
>> probably be usefull to intel instead of having i915 have it's own
>> debugfs file stuff.
>>
>> The lockup dumping public api have been move into radeon_drm.h
>>
>> Stressing the fact again that dump are self contained ie they have
>> all the data needed to be replayed (vertex, indices, shader, texture,
>> ...).
>>
>> Would really like to get this into 3.5, the new API is pretty much
>> straightforward and userspace tools can easily be made to convert
>> it to other format. The change to the driver is self contained.
>
> I really don't like introducing this at this stage into 3.5,
>
> I'd really like a good review of the API and what information we provide
> along with how extensible it is.
>
> I'm still not convinced replay is what we want in the field, I know its what
> *you* want, but I think apitrace stuff in userspace pretty much covers
> the replaying situation. So I'd have to look at this and see how easy
> it makes disecting command streams etc.
>
> Dave.

It store pciid and allow to dump all ib per ring, and all associated
bo object. It also have a bunch of flags to help the userspace tools
(like does userspace need to clear offset (vm vs no vm) ...  What more
do you want to know ?

Cheers,
Jerome

Re: GPU lockup dumping

[Jordan Crouse]

On 05/23/2012 08:51 AM, Jerome Glisse wrote:
> On Wed, May 23, 2012 at 5:27 AM, Dave Airlie<airlied@gmail.com>  wrote:
>> On Thu, May 17, 2012 at 7:28 PM,<j.glisse@gmail.com>  wrote:
>>> So here is improved patchset, where i splited ground work necessary
>>> for the dumping into their own patch. The debugfs improvement could
>>> probably be usefull to intel instead of having i915 have it's own
>>> debugfs file stuff.
>>>
>>> The lockup dumping public api have been move into radeon_drm.h
>>>
>>> Stressing the fact again that dump are self contained ie they have
>>> all the data needed to be replayed (vertex, indices, shader, texture,
>>> ...).
>>>
>>> Would really like to get this into 3.5, the new API is pretty much
>>> straightforward and userspace tools can easily be made to convert
>>> it to other format. The change to the driver is self contained.
>>
>> I really don't like introducing this at this stage into 3.5,
>>
>> I'd really like a good review of the API and what information we provide
>> along with how extensible it is.
>>
>> I'm still not convinced replay is what we want in the field, I know its what
>> *you* want, but I think apitrace stuff in userspace pretty much covers
>> the replaying situation. So I'd have to look at this and see how easy
>> it makes disecting command streams etc.
>>
>> Dave.
>
> It store pciid and allow to dump all ib per ring, and all associated
> bo object. It also have a bunch of flags to help the userspace tools
> (like does userspace need to clear offset (vm vs no vm) ...  What more
> do you want to know ?

Another useful thing might be current register states. We've been doing
dumping (we call it snapshot) in the Qualcomm driver for a little bit now and
between registers, rings, command buffers and various buffers we've been able
to get a reasonably good picture of the state suitable for playback on emulators
and other silly userspace tricks.

We have structs for registers and index/data register pairs because we also dump
lots of debug registers and queues and other various HW sources.

The implementation is way different for obvious reasons but I would love to
consolidate on a single format. Its easy for us to do since we share similar
architectures, but if  least two GPUs support the same format it can be a catalyst
for others to join.

https://www.codeaurora.org/gitweb/quic/la/?p=kernel/msm.git;a=blob;f=drivers/gpu/msm/kgsl_snapshot.h;hb=refs/heads/msm-3.0

Jordan

GPU lockup dumping

[j.glisse]

Make the format more future proof reliable by adding a total chunk
size field that allow old userspace to skip over potentialy new
chunk. Not sure this is really needed but hey.

Jerome

GPU lockup dumping

[j.glisse]

Ok this time is final version, i added a bunch of flags to cmd buffer
to make the userspace tools life easier.

Cheers,
Jerome

Re: GPU lockup dumping

[Jerome Glisse]

On Thu, May 17, 2012 at 6:07 PM,  <j.glisse@gmail.com> wrote:
> Ok this time is final version, i added a bunch of flags to cmd buffer
> to make the userspace tools life easier.
>
> Cheers,
> Jerome
>

So updated libdrm patch :
http://people.freedesktop.org/~glisse/lockup/0001-radeon-add-rati-dumping-helper.patch

Also updated git://people.freedesktop.org/~glisse/joujou and now you
can replay a gpu lockup file captured with those patch on r6xx hw.

Example captured from the kernel (i manualy edited the file to fix
what was causing the lockup so you can replay it safely).
http://people.freedesktop.org/~glisse/lockup/lockup-dump.rati

Also improved the text conversion see :

http://people.freedesktop.org/~glisse/lockup/lockup-dump.tati

Will do r7xx and the evergreen/cayman. But i do think that the kernel
patch are good as is.

Cheers,
Jerome