[PATCH 0/5] drm/ttm: Introduce TTM res manager debugfs helpers

[PATCH 0/5] drm/ttm: Introduce TTM res manager debugfs helpers

[Zack Rusin]

From: Zack Rusin <zackr@vmware.com>

This series introduces generic TTM resource manager debugfs helpers and
refactors TTM drivers which have been using hand rolled out versions
of those to use the new code.

Because those entries are managed by TTM the location of them moves to
/sys/kernel/debug/ttm/. If there are any scripts which depend on the old
locations we can certainly add a new root dentry to
ttm_resource_manager_debugfs_init to preserve the old locations.

Zack Rusin (5):
  drm/ttm: Add common debugfs code for resource managers
  drm/vmwgfx: Add debugfs entries for various ttm resource managers
  drm/amdgpu: Use TTM builtin resource manager debugfs code
  drm/qxl: Use TTM builtin resource manager debugfs code
  drm/radeon: Use TTM builtin resource manager debugfs code

 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 81 ++++---------------------
 drivers/gpu/drm/qxl/qxl_ttm.c           | 40 +++---------
 drivers/gpu/drm/radeon/radeon_ttm.c     | 37 +++--------
 drivers/gpu/drm/ttm/ttm_resource.c      | 65 ++++++++++++++++++++
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     | 10 +++
 include/drm/ttm/ttm_resource.h          |  4 ++
 6 files changed, 104 insertions(+), 133 deletions(-)

-- 
2.32.0

[PATCH 1/5] drm/ttm: Add common debugfs code for resource managers

[Zack Rusin]

From: Zack Rusin <zackr@vmware.com>

Drivers duplicate the code required to add debugfs entries for various
ttm resource managers. To fix it add common TTM resource manager
code that each driver can reuse.

Because TTM resource managers can be initialized and set a lot later
than TTM device initialization a seperate init function is required.
Specific resource managers can overwrite
ttm_resource_manager_func::debug to get more information from those
debugfs entries.

Signed-off-by: Zack Rusin <zackr@vmware.com>
Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Huang Rui <ray.huang@amd.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/ttm/ttm_resource.c | 65 ++++++++++++++++++++++++++++++
 include/drm/ttm/ttm_resource.h     |  4 ++
 2 files changed, 69 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 492ba3157e75..6392ad3e9a88 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -29,6 +29,8 @@
 #include <drm/ttm/ttm_resource.h>
 #include <drm/ttm/ttm_bo_driver.h>
 
+#include "ttm_module.h"
+
 /**
  * ttm_lru_bulk_move_init - initialize a bulk move structure
  * @bulk: the structure to init
@@ -644,3 +646,66 @@ ttm_kmap_iter_linear_io_fini(struct ttm_kmap_iter_linear_io *iter_io,
 
 	ttm_mem_io_free(bdev, mem);
 }
+
+#if defined(CONFIG_DEBUG_FS)
+
+#define TTM_RES_MAN_SHOW(i) \
+	static int ttm_resource_manager##i##_show(struct seq_file *m, void *unused) \
+	{ \
+		struct ttm_device *bdev = (struct ttm_device *)m->private; \
+		struct ttm_resource_manager *man = ttm_manager_type(bdev, i); \
+		struct drm_printer p = drm_seq_file_printer(m); \
+		ttm_resource_manager_debug(man, &p); \
+		return 0; \
+	}\
+	DEFINE_SHOW_ATTRIBUTE(ttm_resource_manager##i)
+
+TTM_RES_MAN_SHOW(0);
+TTM_RES_MAN_SHOW(1);
+TTM_RES_MAN_SHOW(2);
+TTM_RES_MAN_SHOW(3);
+TTM_RES_MAN_SHOW(4);
+TTM_RES_MAN_SHOW(5);
+TTM_RES_MAN_SHOW(6);
+
+#endif
+
+/**
+ * ttm_resource_manager_debugfs_init - Setup debugfs entries for specified
+ * resource managers.
+ * @bdev: The TTM device
+ * @file_names: A mapping between TTM_TT placements and the debugfs file
+ * names
+ * @num_file_names: The array size of @file_names.
+ *
+ * This function setups up debugfs files that can be used to look
+ * at debug statistics of the specified ttm_resource_managers.
+ * @file_names array is used to figure out which ttm placements
+ * will get debugfs files created for them.
+ */
+void
+ttm_resource_manager_debugfs_init(struct ttm_device *bdev,
+				  const char * const file_names[],
+				  uint32_t num_file_names)
+{
+#if defined(CONFIG_DEBUG_FS)
+	uint32_t i;
+	const struct file_operations *fops[] = {
+		&ttm_resource_manager0_fops,
+		&ttm_resource_manager1_fops,
+		&ttm_resource_manager2_fops,
+		&ttm_resource_manager3_fops,
+		&ttm_resource_manager4_fops,
+		&ttm_resource_manager5_fops,
+		&ttm_resource_manager6_fops,
+	};
+
+	WARN_ON(num_file_names > ARRAY_SIZE(fops));
+
+	for (i = 0; i < num_file_names; ++i)
+		if (file_names[i] && fops[i])
+			debugfs_create_file(file_names[i], 0444,
+					    ttm_debugfs_root, bdev, fops[i]);
+#endif
+}
+EXPORT_SYMBOL(ttm_resource_manager_debugfs_init);
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index 4428a62e5f0e..3c85cdd21ca5 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -383,4 +383,8 @@ ttm_kmap_iter_linear_io_init(struct ttm_kmap_iter_linear_io *iter_io,
 void ttm_kmap_iter_linear_io_fini(struct ttm_kmap_iter_linear_io *iter_io,
 				  struct ttm_device *bdev,
 				  struct ttm_resource *mem);
+
+void ttm_resource_manager_debugfs_init(struct ttm_device *bdev,
+				       const char * const file_name[],
+				       uint32_t num_file_names);
 #endif
-- 
2.32.0

[PATCH 2/5] drm/vmwgfx: Add debugfs entries for various ttm resource managers

[Zack Rusin]

From: Zack Rusin <zackr@vmware.com>

Use the newly added TTM's ability to automatically create debugfs entries
for specified placements. This creates entries in /sys/kernel/debug/ttm/
that can be read to get information about various TTM resource
managers which are used by vmwgfx.

Signed-off-by: Zack Rusin <zackr@vmware.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index decd54b8333d..59d0d1cd564b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1630,6 +1630,13 @@ static int vmw_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct vmw_private *vmw;
 	int ret;
+	const char * const ttm_placement_names[] = {
+		[TTM_PL_SYSTEM] = "system_ttm",
+		[TTM_PL_VRAM] = "vram_ttm",
+		[VMW_PL_GMR] = "gmr_ttm",
+		[VMW_PL_MOB] = "mob_ttm",
+		[VMW_PL_SYSTEM] = "system_mob_ttm"
+	};
 
 	ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &driver);
 	if (ret)
@@ -1657,6 +1664,9 @@ static int vmw_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto out_unload;
 
 	vmw_debugfs_gem_init(vmw);
+	ttm_resource_manager_debugfs_init(&vmw->bdev,
+					  ttm_placement_names,
+					  ARRAY_SIZE(ttm_placement_names));
 
 	return 0;
 out_unload:
-- 
2.32.0

[PATCH 3/5] drm/amdgpu: Use TTM builtin resource manager debugfs code

[Zack Rusin]

From: Zack Rusin <zackr@vmware.com>

Switch to using the TTM resource manager debugfs helpers. It's
exactly the same functionality, except that the entries live under
/sys/kernel/debug/ttm/.

Signed-off-by: Zack Rusin <zackr@vmware.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Felix Kuehling <Felix.Kuehling@amd.com>
Cc: Nirmoy Das <nirmoy.das@amd.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 81 ++++---------------------
 1 file changed, 11 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 57ac118fc266..9852a11a5758 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2079,17 +2079,6 @@ int amdgpu_ttm_evict_resources(struct amdgpu_device *adev, int mem_type)
 
 #if defined(CONFIG_DEBUG_FS)
 
-static int amdgpu_mm_vram_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    TTM_PL_VRAM);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
 static int amdgpu_ttm_page_pool_show(struct seq_file *m, void *unused)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
@@ -2097,55 +2086,6 @@ static int amdgpu_ttm_page_pool_show(struct seq_file *m, void *unused)
 	return ttm_pool_debugfs(&adev->mman.bdev.pool, m);
 }
 
-static int amdgpu_mm_tt_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    TTM_PL_TT);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-static int amdgpu_mm_gds_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    AMDGPU_PL_GDS);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-static int amdgpu_mm_gws_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    AMDGPU_PL_GWS);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-static int amdgpu_mm_oa_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    AMDGPU_PL_OA);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_vram_table);
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_tt_table);
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_gds_table);
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_gws_table);
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_oa_table);
 DEFINE_SHOW_ATTRIBUTE(amdgpu_ttm_page_pool);
 
 /*
@@ -2350,22 +2290,23 @@ void amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
 #if defined(CONFIG_DEBUG_FS)
 	struct drm_minor *minor = adev_to_drm(adev)->primary;
 	struct dentry *root = minor->debugfs_root;
+	const char * const ttm_placement_names[] = {
+		[TTM_PL_SYSTEM] = "system_ttm",
+		[TTM_PL_VRAM] = "amdgpu_vram_mm",
+		[TTM_PL_TT] = "amdgpu_gtt_mm",
+		[AMDGPU_PL_GDS] = "amdgpu_gds_mm",
+		[AMDGPU_PL_GWS] = "amdgpu_gws_mm",
+		[AMDGPU_PL_OA] = "amdgpu_oa_mm"
+	};
 
 	debugfs_create_file_size("amdgpu_vram", 0444, root, adev,
 				 &amdgpu_ttm_vram_fops, adev->gmc.mc_vram_size);
 	debugfs_create_file("amdgpu_iomem", 0444, root, adev,
 			    &amdgpu_ttm_iomem_fops);
-	debugfs_create_file("amdgpu_vram_mm", 0444, root, adev,
-			    &amdgpu_mm_vram_table_fops);
-	debugfs_create_file("amdgpu_gtt_mm", 0444, root, adev,
-			    &amdgpu_mm_tt_table_fops);
-	debugfs_create_file("amdgpu_gds_mm", 0444, root, adev,
-			    &amdgpu_mm_gds_table_fops);
-	debugfs_create_file("amdgpu_gws_mm", 0444, root, adev,
-			    &amdgpu_mm_gws_table_fops);
-	debugfs_create_file("amdgpu_oa_mm", 0444, root, adev,
-			    &amdgpu_mm_oa_table_fops);
 	debugfs_create_file("ttm_page_pool", 0444, root, adev,
 			    &amdgpu_ttm_page_pool_fops);
+	ttm_resource_manager_debugfs_init(&adev->mman.bdev,
+					  ttm_placement_names,
+					  ARRAY_SIZE(ttm_placement_names));
 #endif
 }
-- 
2.32.0

[PATCH 3/5] drm/amdgpu: Use TTM builtin resource manager debugfs code

[Zack Rusin]

From: Zack Rusin <zackr@vmware.com>

Switch to using the TTM resource manager debugfs helpers. It's
exactly the same functionality, except that the entries live under
/sys/kernel/debug/ttm/.

Signed-off-by: Zack Rusin <zackr@vmware.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Felix Kuehling <Felix.Kuehling@amd.com>
Cc: Nirmoy Das <nirmoy.das@amd.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 81 ++++---------------------
 1 file changed, 11 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 57ac118fc266..9852a11a5758 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2079,17 +2079,6 @@ int amdgpu_ttm_evict_resources(struct amdgpu_device *adev, int mem_type)
 
 #if defined(CONFIG_DEBUG_FS)
 
-static int amdgpu_mm_vram_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    TTM_PL_VRAM);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
 static int amdgpu_ttm_page_pool_show(struct seq_file *m, void *unused)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
@@ -2097,55 +2086,6 @@ static int amdgpu_ttm_page_pool_show(struct seq_file *m, void *unused)
 	return ttm_pool_debugfs(&adev->mman.bdev.pool, m);
 }
 
-static int amdgpu_mm_tt_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    TTM_PL_TT);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-static int amdgpu_mm_gds_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    AMDGPU_PL_GDS);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-static int amdgpu_mm_gws_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    AMDGPU_PL_GWS);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-static int amdgpu_mm_oa_table_show(struct seq_file *m, void *unused)
-{
-	struct amdgpu_device *adev = (struct amdgpu_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&adev->mman.bdev,
-							    AMDGPU_PL_OA);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_vram_table);
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_tt_table);
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_gds_table);
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_gws_table);
-DEFINE_SHOW_ATTRIBUTE(amdgpu_mm_oa_table);
 DEFINE_SHOW_ATTRIBUTE(amdgpu_ttm_page_pool);
 
 /*
@@ -2350,22 +2290,23 @@ void amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
 #if defined(CONFIG_DEBUG_FS)
 	struct drm_minor *minor = adev_to_drm(adev)->primary;
 	struct dentry *root = minor->debugfs_root;
+	const char * const ttm_placement_names[] = {
+		[TTM_PL_SYSTEM] = "system_ttm",
+		[TTM_PL_VRAM] = "amdgpu_vram_mm",
+		[TTM_PL_TT] = "amdgpu_gtt_mm",
+		[AMDGPU_PL_GDS] = "amdgpu_gds_mm",
+		[AMDGPU_PL_GWS] = "amdgpu_gws_mm",
+		[AMDGPU_PL_OA] = "amdgpu_oa_mm"
+	};
 
 	debugfs_create_file_size("amdgpu_vram", 0444, root, adev,
 				 &amdgpu_ttm_vram_fops, adev->gmc.mc_vram_size);
 	debugfs_create_file("amdgpu_iomem", 0444, root, adev,
 			    &amdgpu_ttm_iomem_fops);
-	debugfs_create_file("amdgpu_vram_mm", 0444, root, adev,
-			    &amdgpu_mm_vram_table_fops);
-	debugfs_create_file("amdgpu_gtt_mm", 0444, root, adev,
-			    &amdgpu_mm_tt_table_fops);
-	debugfs_create_file("amdgpu_gds_mm", 0444, root, adev,
-			    &amdgpu_mm_gds_table_fops);
-	debugfs_create_file("amdgpu_gws_mm", 0444, root, adev,
-			    &amdgpu_mm_gws_table_fops);
-	debugfs_create_file("amdgpu_oa_mm", 0444, root, adev,
-			    &amdgpu_mm_oa_table_fops);
 	debugfs_create_file("ttm_page_pool", 0444, root, adev,
 			    &amdgpu_ttm_page_pool_fops);
+	ttm_resource_manager_debugfs_init(&adev->mman.bdev,
+					  ttm_placement_names,
+					  ARRAY_SIZE(ttm_placement_names));
 #endif
 }
-- 
2.32.0

[PATCH 4/5] drm/qxl: Use TTM builtin resource manager debugfs code

[Zack Rusin]

From: Zack Rusin <zackr@vmware.com>

Switch to using the TTM resource manager debugfs helpers. The
functionality is largely the same, except that the entries live under
/sys/kernel/debug/ttm/ and their lifetimes are managed by TTM.

Signed-off-by: Zack Rusin <zackr@vmware.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: virtualization@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 40 ++++++-----------------------------
 1 file changed, 7 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 95df5750f47f..c31d4d751dc4 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -222,41 +222,15 @@ void qxl_ttm_fini(struct qxl_device *qdev)
 	DRM_INFO("qxl: ttm finalized\n");
 }
 
-#define QXL_DEBUGFS_MEM_TYPES 2
-
-#if defined(CONFIG_DEBUG_FS)
-static int qxl_mm_dump_table(struct seq_file *m, void *data)
-{
-	struct drm_info_node *node = (struct drm_info_node *)m->private;
-	struct ttm_resource_manager *man = (struct ttm_resource_manager *)node->info_ent->data;
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-#endif
-
 void qxl_ttm_debugfs_init(struct qxl_device *qdev)
 {
 #if defined(CONFIG_DEBUG_FS)
-	static struct drm_info_list qxl_mem_types_list[QXL_DEBUGFS_MEM_TYPES];
-	static char qxl_mem_types_names[QXL_DEBUGFS_MEM_TYPES][32];
-	unsigned int i;
-
-	for (i = 0; i < QXL_DEBUGFS_MEM_TYPES; i++) {
-		if (i == 0)
-			sprintf(qxl_mem_types_names[i], "qxl_mem_mm");
-		else
-			sprintf(qxl_mem_types_names[i], "qxl_surf_mm");
-		qxl_mem_types_list[i].name = qxl_mem_types_names[i];
-		qxl_mem_types_list[i].show = &qxl_mm_dump_table;
-		qxl_mem_types_list[i].driver_features = 0;
-		if (i == 0)
-			qxl_mem_types_list[i].data = ttm_manager_type(&qdev->mman.bdev, TTM_PL_VRAM);
-		else
-			qxl_mem_types_list[i].data = ttm_manager_type(&qdev->mman.bdev, TTM_PL_PRIV);
-
-	}
-	qxl_debugfs_add_files(qdev, qxl_mem_types_list, i);
+	const char * const ttm_placement_names[] = {
+		[TTM_PL_VRAM] = "qxl_mem_mm",
+		[TTM_PL_PRIV] = "qxl_surf_mm",
+	};
+	ttm_resource_manager_debugfs_init(&qdev->mman.bdev,
+					  ttm_placement_names,
+					  ARRAY_SIZE(ttm_placement_names));
 #endif
 }
-- 
2.32.0

[PATCH 5/5] drm/radeon: Use TTM builtin resource manager debugfs code

[Zack Rusin]

From: Zack Rusin <zackr@vmware.com>

Switch to using the TTM resource manager debugfs helpers. The
functionality is largely the same, except that the entries live under
/sys/kernel/debug/ttm/ and their lifetimes are managed by TTM.

Signed-off-by: Zack Rusin <zackr@vmware.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 37 ++++++-----------------------
 1 file changed, 7 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 44594d16611f..dc6250d5a9cf 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -781,17 +781,6 @@ void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size)
 
 #if defined(CONFIG_DEBUG_FS)
 
-static int radeon_mm_vram_dump_table_show(struct seq_file *m, void *unused)
-{
-	struct radeon_device *rdev = (struct radeon_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&rdev->mman.bdev,
-							    TTM_PL_VRAM);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
 static int radeon_ttm_page_pool_show(struct seq_file *m, void *data)
 {
 	struct radeon_device *rdev = (struct radeon_device *)m->private;
@@ -799,19 +788,6 @@ static int radeon_ttm_page_pool_show(struct seq_file *m, void *data)
 	return ttm_pool_debugfs(&rdev->mman.bdev.pool, m);
 }
 
-static int radeon_mm_gtt_dump_table_show(struct seq_file *m, void *unused)
-{
-	struct radeon_device *rdev = (struct radeon_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&rdev->mman.bdev,
-							    TTM_PL_TT);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-DEFINE_SHOW_ATTRIBUTE(radeon_mm_vram_dump_table);
-DEFINE_SHOW_ATTRIBUTE(radeon_mm_gtt_dump_table);
 DEFINE_SHOW_ATTRIBUTE(radeon_ttm_page_pool);
 
 static int radeon_ttm_vram_open(struct inode *inode, struct file *filep)
@@ -927,18 +903,19 @@ static void radeon_ttm_debugfs_init(struct radeon_device *rdev)
 #if defined(CONFIG_DEBUG_FS)
 	struct drm_minor *minor = rdev->ddev->primary;
 	struct dentry *root = minor->debugfs_root;
+	const char * const ttm_placement_names[] = {
+		[TTM_PL_TT] = "radeon_gtt_mm",
+		[TTM_PL_VRAM] = "radeon_vram_mm"
+	};
 
 	debugfs_create_file("radeon_vram", 0444, root, rdev,
 			    &radeon_ttm_vram_fops);
-
 	debugfs_create_file("radeon_gtt", 0444, root, rdev,
 			    &radeon_ttm_gtt_fops);
-
-	debugfs_create_file("radeon_vram_mm", 0444, root, rdev,
-			    &radeon_mm_vram_dump_table_fops);
-	debugfs_create_file("radeon_gtt_mm", 0444, root, rdev,
-			    &radeon_mm_gtt_dump_table_fops);
 	debugfs_create_file("ttm_page_pool", 0444, root, rdev,
 			    &radeon_ttm_page_pool_fops);
+	ttm_resource_manager_debugfs_init(&rdev->mman.bdev,
+					  ttm_placement_names,
+					  ARRAY_SIZE(ttm_placement_names));
 #endif
 }
-- 
2.32.0

[PATCH 5/5] drm/radeon: Use TTM builtin resource manager debugfs code

[Zack Rusin]

From: Zack Rusin <zackr@vmware.com>

Switch to using the TTM resource manager debugfs helpers. The
functionality is largely the same, except that the entries live under
/sys/kernel/debug/ttm/ and their lifetimes are managed by TTM.

Signed-off-by: Zack Rusin <zackr@vmware.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "Pan, Xinhui" <Xinhui.Pan@amd.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 37 ++++++-----------------------
 1 file changed, 7 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 44594d16611f..dc6250d5a9cf 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -781,17 +781,6 @@ void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size)
 
 #if defined(CONFIG_DEBUG_FS)
 
-static int radeon_mm_vram_dump_table_show(struct seq_file *m, void *unused)
-{
-	struct radeon_device *rdev = (struct radeon_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&rdev->mman.bdev,
-							    TTM_PL_VRAM);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
 static int radeon_ttm_page_pool_show(struct seq_file *m, void *data)
 {
 	struct radeon_device *rdev = (struct radeon_device *)m->private;
@@ -799,19 +788,6 @@ static int radeon_ttm_page_pool_show(struct seq_file *m, void *data)
 	return ttm_pool_debugfs(&rdev->mman.bdev.pool, m);
 }
 
-static int radeon_mm_gtt_dump_table_show(struct seq_file *m, void *unused)
-{
-	struct radeon_device *rdev = (struct radeon_device *)m->private;
-	struct ttm_resource_manager *man = ttm_manager_type(&rdev->mman.bdev,
-							    TTM_PL_TT);
-	struct drm_printer p = drm_seq_file_printer(m);
-
-	ttm_resource_manager_debug(man, &p);
-	return 0;
-}
-
-DEFINE_SHOW_ATTRIBUTE(radeon_mm_vram_dump_table);
-DEFINE_SHOW_ATTRIBUTE(radeon_mm_gtt_dump_table);
 DEFINE_SHOW_ATTRIBUTE(radeon_ttm_page_pool);
 
 static int radeon_ttm_vram_open(struct inode *inode, struct file *filep)
@@ -927,18 +903,19 @@ static void radeon_ttm_debugfs_init(struct radeon_device *rdev)
 #if defined(CONFIG_DEBUG_FS)
 	struct drm_minor *minor = rdev->ddev->primary;
 	struct dentry *root = minor->debugfs_root;
+	const char * const ttm_placement_names[] = {
+		[TTM_PL_TT] = "radeon_gtt_mm",
+		[TTM_PL_VRAM] = "radeon_vram_mm"
+	};
 
 	debugfs_create_file("radeon_vram", 0444, root, rdev,
 			    &radeon_ttm_vram_fops);
-
 	debugfs_create_file("radeon_gtt", 0444, root, rdev,
 			    &radeon_ttm_gtt_fops);
-
-	debugfs_create_file("radeon_vram_mm", 0444, root, rdev,
-			    &radeon_mm_vram_dump_table_fops);
-	debugfs_create_file("radeon_gtt_mm", 0444, root, rdev,
-			    &radeon_mm_gtt_dump_table_fops);
 	debugfs_create_file("ttm_page_pool", 0444, root, rdev,
 			    &radeon_ttm_page_pool_fops);
+	ttm_resource_manager_debugfs_init(&rdev->mman.bdev,
+					  ttm_placement_names,
+					  ARRAY_SIZE(ttm_placement_names));
 #endif
 }
-- 
2.32.0

Re: [PATCH 1/5] drm/ttm: Add common debugfs code for resource managers

[Christian König]

Am 07.04.22 um 04:56 schrieb Zack Rusin:
> From: Zack Rusin <zackr@vmware.com>
>
> Drivers duplicate the code required to add debugfs entries for various
> ttm resource managers. To fix it add common TTM resource manager
> code that each driver can reuse.
>
> Because TTM resource managers can be initialized and set a lot later
> than TTM device initialization a seperate init function is required.
> Specific resource managers can overwrite
> ttm_resource_manager_func::debug to get more information from those
> debugfs entries.
>
> Signed-off-by: Zack Rusin <zackr@vmware.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Cc: Huang Rui <ray.huang@amd.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>

Ah, yes that was on my TODO list for quite a while as well.

> ---
>   drivers/gpu/drm/ttm/ttm_resource.c | 65 ++++++++++++++++++++++++++++++
>   include/drm/ttm/ttm_resource.h     |  4 ++
>   2 files changed, 69 insertions(+)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
> index 492ba3157e75..6392ad3e9a88 100644
> --- a/drivers/gpu/drm/ttm/ttm_resource.c
> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> @@ -29,6 +29,8 @@
>   #include <drm/ttm/ttm_resource.h>
>   #include <drm/ttm/ttm_bo_driver.h>
>   
> +#include "ttm_module.h"
> +
>   /**
>    * ttm_lru_bulk_move_init - initialize a bulk move structure
>    * @bulk: the structure to init
> @@ -644,3 +646,66 @@ ttm_kmap_iter_linear_io_fini(struct ttm_kmap_iter_linear_io *iter_io,
>   
>   	ttm_mem_io_free(bdev, mem);
>   }
> +
> +#if defined(CONFIG_DEBUG_FS)
> +
> +#define TTM_RES_MAN_SHOW(i) \
> +	static int ttm_resource_manager##i##_show(struct seq_file *m, void *unused) \
> +	{ \
> +		struct ttm_device *bdev = (struct ttm_device *)m->private; \
> +		struct ttm_resource_manager *man = ttm_manager_type(bdev, i); \
> +		struct drm_printer p = drm_seq_file_printer(m); \
> +		ttm_resource_manager_debug(man, &p); \
> +		return 0; \
> +	}\
> +	DEFINE_SHOW_ATTRIBUTE(ttm_resource_manager##i)
> +
> +TTM_RES_MAN_SHOW(0);
> +TTM_RES_MAN_SHOW(1);
> +TTM_RES_MAN_SHOW(2);
> +TTM_RES_MAN_SHOW(3);
> +TTM_RES_MAN_SHOW(4);
> +TTM_RES_MAN_SHOW(5);
> +TTM_RES_MAN_SHOW(6);

Uff, please not a static array.

> +
> +#endif
> +
> +/**
> + * ttm_resource_manager_debugfs_init - Setup debugfs entries for specified
> + * resource managers.
> + * @bdev: The TTM device
> + * @file_names: A mapping between TTM_TT placements and the debugfs file
> + * names
> + * @num_file_names: The array size of @file_names.
> + *
> + * This function setups up debugfs files that can be used to look
> + * at debug statistics of the specified ttm_resource_managers.
> + * @file_names array is used to figure out which ttm placements
> + * will get debugfs files created for them.
> + */
> +void
> +ttm_resource_manager_debugfs_init(struct ttm_device *bdev,
> +				  const char * const file_names[],
> +				  uint32_t num_file_names)
> +{
> +#if defined(CONFIG_DEBUG_FS)
> +	uint32_t i;
> +	const struct file_operations *fops[] = {
> +		&ttm_resource_manager0_fops,
> +		&ttm_resource_manager1_fops,
> +		&ttm_resource_manager2_fops,
> +		&ttm_resource_manager3_fops,
> +		&ttm_resource_manager4_fops,
> +		&ttm_resource_manager5_fops,
> +		&ttm_resource_manager6_fops,
> +	};
> +
> +	WARN_ON(num_file_names > ARRAY_SIZE(fops));
> +
> +	for (i = 0; i < num_file_names; ++i)
> +		if (file_names[i] && fops[i])
> +			debugfs_create_file(file_names[i], 0444,
> +					    ttm_debugfs_root, bdev, fops[i]);

You can give the ttm_resource_manager directly as parameter here instead 
of the bdev and avoid the whole handling with the macro and global arrays.

Then ttm_debugfs_root is the global directory for TTM and not meant to 
be used for driver specific data.

Rather do it like this: void ttm_resource_manager_create_debugfs(struct 
ttm_resource_manager *man, struct dentry * parent, const char *name);

Calling that for each file the driver wants to create should be trivial.

Thanks,
Christian.

> +#endif
> +}
> +EXPORT_SYMBOL(ttm_resource_manager_debugfs_init);
> diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
> index 4428a62e5f0e..3c85cdd21ca5 100644
> --- a/include/drm/ttm/ttm_resource.h
> +++ b/include/drm/ttm/ttm_resource.h
> @@ -383,4 +383,8 @@ ttm_kmap_iter_linear_io_init(struct ttm_kmap_iter_linear_io *iter_io,
>   void ttm_kmap_iter_linear_io_fini(struct ttm_kmap_iter_linear_io *iter_io,
>   				  struct ttm_device *bdev,
>   				  struct ttm_resource *mem);
> +
> +void ttm_resource_manager_debugfs_init(struct ttm_device *bdev,
> +				       const char * const file_name[],
> +				       uint32_t num_file_names);
>   #endif

Re: [PATCH 1/5] drm/ttm: Add common debugfs code for resource managers

[Zack Rusin]

On Thu, 2022-04-07 at 08:01 +0200, Christian König wrote:
> Am 07.04.22 um 04:56 schrieb Zack Rusin:
> > From: Zack Rusin <zackr@vmware.com>
> > 
> > Drivers duplicate the code required to add debugfs entries for
> > various
> > ttm resource managers. To fix it add common TTM resource manager
> > code that each driver can reuse.
> > 
> > Because TTM resource managers can be initialized and set a lot
> > later
> > than TTM device initialization a seperate init function is
> > required.
> > Specific resource managers can overwrite
> > ttm_resource_manager_func::debug to get more information from those
> > debugfs entries.
> > 
> > Signed-off-by: Zack Rusin <zackr@vmware.com>
> > Cc: Christian Koenig <christian.koenig@amd.com>
> > Cc: Huang Rui <ray.huang@amd.com>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> 
> Ah, yes that was on my TODO list for quite a while as well.
> 
> > ---
> >   drivers/gpu/drm/ttm/ttm_resource.c | 65
> > ++++++++++++++++++++++++++++++
> >   include/drm/ttm/ttm_resource.h     |  4 ++
> >   2 files changed, 69 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> > b/drivers/gpu/drm/ttm/ttm_resource.c
> > index 492ba3157e75..6392ad3e9a88 100644
> > --- a/drivers/gpu/drm/ttm/ttm_resource.c
> > +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> > @@ -29,6 +29,8 @@
> >   #include <drm/ttm/ttm_resource.h>
> >   #include <drm/ttm/ttm_bo_driver.h>
> > 
> > +#include "ttm_module.h"
> > +
> >   /**
> >    * ttm_lru_bulk_move_init - initialize a bulk move structure
> >    * @bulk: the structure to init
> > @@ -644,3 +646,66 @@ ttm_kmap_iter_linear_io_fini(struct
> > ttm_kmap_iter_linear_io *iter_io,
> > 
> >       ttm_mem_io_free(bdev, mem);
> >   }
> > +
> > +#if defined(CONFIG_DEBUG_FS)
> > +
> > +#define TTM_RES_MAN_SHOW(i) \
> > +     static int ttm_resource_manager##i##_show(struct seq_file *m,
> > void *unused) \
> > +     { \
> > +             struct ttm_device *bdev = (struct ttm_device *)m-
> > >private; \
> > +             struct ttm_resource_manager *man =
> > ttm_manager_type(bdev, i); \
> > +             struct drm_printer p = drm_seq_file_printer(m); \
> > +             ttm_resource_manager_debug(man, &p); \
> > +             return 0; \
> > +     }\
> > +     DEFINE_SHOW_ATTRIBUTE(ttm_resource_manager##i)
> > +
> > +TTM_RES_MAN_SHOW(0);
> > +TTM_RES_MAN_SHOW(1);
> > +TTM_RES_MAN_SHOW(2);
> > +TTM_RES_MAN_SHOW(3);
> > +TTM_RES_MAN_SHOW(4);
> > +TTM_RES_MAN_SHOW(5);
> > +TTM_RES_MAN_SHOW(6);
> 
> Uff, please not a static array.
> 
> > +
> > +#endif
> > +
> > +/**
> > + * ttm_resource_manager_debugfs_init - Setup debugfs entries for
> > specified
> > + * resource managers.
> > + * @bdev: The TTM device
> > + * @file_names: A mapping between TTM_TT placements and the
> > debugfs file
> > + * names
> > + * @num_file_names: The array size of @file_names.
> > + *
> > + * This function setups up debugfs files that can be used to look
> > + * at debug statistics of the specified ttm_resource_managers.
> > + * @file_names array is used to figure out which ttm placements
> > + * will get debugfs files created for them.
> > + */
> > +void
> > +ttm_resource_manager_debugfs_init(struct ttm_device *bdev,
> > +                               const char * const file_names[],
> > +                               uint32_t num_file_names)
> > +{
> > +#if defined(CONFIG_DEBUG_FS)
> > +     uint32_t i;
> > +     const struct file_operations *fops[] = {
> > +             &ttm_resource_manager0_fops,
> > +             &ttm_resource_manager1_fops,
> > +             &ttm_resource_manager2_fops,
> > +             &ttm_resource_manager3_fops,
> > +             &ttm_resource_manager4_fops,
> > +             &ttm_resource_manager5_fops,
> > +             &ttm_resource_manager6_fops,
> > +     };
> > +
> > +     WARN_ON(num_file_names > ARRAY_SIZE(fops));
> > +
> > +     for (i = 0; i < num_file_names; ++i)
> > +             if (file_names[i] && fops[i])
> > +                     debugfs_create_file(file_names[i], 0444,
> > +                                         ttm_debugfs_root, bdev,
> > fops[i]);
> 
> You can give the ttm_resource_manager directly as parameter here
> instead
> of the bdev and avoid the whole handling with the macro and global
> arrays.

We could but that does change the behaviour. I was trying to preserve
the old semantics. Because the lifetimes of driver specific managers
are not handled by ttm, there's nothing preventing the drivers from,
e.g. during reset, deleting the old and setting up new resource
managers. By always using ttm_manager_type() we make sure that we're
always using the current one. Passing ttm_resource_manager directly
makes it impossible to validate that at _show() time
ttm_resource_manager is still valid. It's not a problem for vmwgfx
because we never reset the resource managers but I didn't feel
comfortable changing the semantics like that for all drivers. It could
lead to weird crashes, but if you prefer that approach I'm happy to
change it.

z

Re: [PATCH 1/5] drm/ttm: Add common debugfs code for resource managers

[Daniel Vetter]

On Thu, Apr 07, 2022 at 02:15:52PM +0000, Zack Rusin wrote:
> On Thu, 2022-04-07 at 08:01 +0200, Christian König wrote:
> > Am 07.04.22 um 04:56 schrieb Zack Rusin:
> > > From: Zack Rusin <zackr@vmware.com>
> > > 
> > > Drivers duplicate the code required to add debugfs entries for
> > > various
> > > ttm resource managers. To fix it add common TTM resource manager
> > > code that each driver can reuse.
> > > 
> > > Because TTM resource managers can be initialized and set a lot
> > > later
> > > than TTM device initialization a seperate init function is
> > > required.
> > > Specific resource managers can overwrite
> > > ttm_resource_manager_func::debug to get more information from those
> > > debugfs entries.
> > > 
> > > Signed-off-by: Zack Rusin <zackr@vmware.com>
> > > Cc: Christian Koenig <christian.koenig@amd.com>
> > > Cc: Huang Rui <ray.huang@amd.com>
> > > Cc: David Airlie <airlied@linux.ie>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > 
> > Ah, yes that was on my TODO list for quite a while as well.
> > 
> > > ---
> > >   drivers/gpu/drm/ttm/ttm_resource.c | 65
> > > ++++++++++++++++++++++++++++++
> > >   include/drm/ttm/ttm_resource.h     |  4 ++
> > >   2 files changed, 69 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
> > > b/drivers/gpu/drm/ttm/ttm_resource.c
> > > index 492ba3157e75..6392ad3e9a88 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_resource.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_resource.c
> > > @@ -29,6 +29,8 @@
> > >   #include <drm/ttm/ttm_resource.h>
> > >   #include <drm/ttm/ttm_bo_driver.h>
> > > 
> > > +#include "ttm_module.h"
> > > +
> > >   /**
> > >    * ttm_lru_bulk_move_init - initialize a bulk move structure
> > >    * @bulk: the structure to init
> > > @@ -644,3 +646,66 @@ ttm_kmap_iter_linear_io_fini(struct
> > > ttm_kmap_iter_linear_io *iter_io,
> > > 
> > >       ttm_mem_io_free(bdev, mem);
> > >   }
> > > +
> > > +#if defined(CONFIG_DEBUG_FS)
> > > +
> > > +#define TTM_RES_MAN_SHOW(i) \
> > > +     static int ttm_resource_manager##i##_show(struct seq_file *m,
> > > void *unused) \
> > > +     { \
> > > +             struct ttm_device *bdev = (struct ttm_device *)m-
> > > >private; \
> > > +             struct ttm_resource_manager *man =
> > > ttm_manager_type(bdev, i); \
> > > +             struct drm_printer p = drm_seq_file_printer(m); \
> > > +             ttm_resource_manager_debug(man, &p); \
> > > +             return 0; \
> > > +     }\
> > > +     DEFINE_SHOW_ATTRIBUTE(ttm_resource_manager##i)
> > > +
> > > +TTM_RES_MAN_SHOW(0);
> > > +TTM_RES_MAN_SHOW(1);
> > > +TTM_RES_MAN_SHOW(2);
> > > +TTM_RES_MAN_SHOW(3);
> > > +TTM_RES_MAN_SHOW(4);
> > > +TTM_RES_MAN_SHOW(5);
> > > +TTM_RES_MAN_SHOW(6);
> > 
> > Uff, please not a static array.
> > 
> > > +
> > > +#endif
> > > +
> > > +/**
> > > + * ttm_resource_manager_debugfs_init - Setup debugfs entries for
> > > specified
> > > + * resource managers.
> > > + * @bdev: The TTM device
> > > + * @file_names: A mapping between TTM_TT placements and the
> > > debugfs file
> > > + * names
> > > + * @num_file_names: The array size of @file_names.
> > > + *
> > > + * This function setups up debugfs files that can be used to look
> > > + * at debug statistics of the specified ttm_resource_managers.
> > > + * @file_names array is used to figure out which ttm placements
> > > + * will get debugfs files created for them.
> > > + */
> > > +void
> > > +ttm_resource_manager_debugfs_init(struct ttm_device *bdev,
> > > +                               const char * const file_names[],
> > > +                               uint32_t num_file_names)
> > > +{
> > > +#if defined(CONFIG_DEBUG_FS)
> > > +     uint32_t i;
> > > +     const struct file_operations *fops[] = {
> > > +             &ttm_resource_manager0_fops,
> > > +             &ttm_resource_manager1_fops,
> > > +             &ttm_resource_manager2_fops,
> > > +             &ttm_resource_manager3_fops,
> > > +             &ttm_resource_manager4_fops,
> > > +             &ttm_resource_manager5_fops,
> > > +             &ttm_resource_manager6_fops,
> > > +     };
> > > +
> > > +     WARN_ON(num_file_names > ARRAY_SIZE(fops));
> > > +
> > > +     for (i = 0; i < num_file_names; ++i)
> > > +             if (file_names[i] && fops[i])
> > > +                     debugfs_create_file(file_names[i], 0444,
> > > +                                         ttm_debugfs_root, bdev,
> > > fops[i]);
> > 
> > You can give the ttm_resource_manager directly as parameter here
> > instead
> > of the bdev and avoid the whole handling with the macro and global
> > arrays.
> 
> We could but that does change the behaviour. I was trying to preserve
> the old semantics. Because the lifetimes of driver specific managers
> are not handled by ttm, there's nothing preventing the drivers from,
> e.g. during reset, deleting the old and setting up new resource
> managers. By always using ttm_manager_type() we make sure that we're
> always using the current one. Passing ttm_resource_manager directly
> makes it impossible to validate that at _show() time
> ttm_resource_manager is still valid. It's not a problem for vmwgfx
> because we never reset the resource managers but I didn't feel
> comfortable changing the semantics like that for all drivers. It could
> lead to weird crashes, but if you prefer that approach I'm happy to
> change it.

Uh resetting resource managers during the lifetime of a drm_device sounds
very broken. I guess it's somewhat ok over suspend/resume or so when
userspace cannot access the driver, but even then it smells fishy.

Unless we have a driver doing that I don't think this is a use-case we
should support.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/5] drm/ttm: Add common debugfs code for resource managers

[Christian König]

Am 08.04.22 um 09:56 schrieb Daniel Vetter:
> On Thu, Apr 07, 2022 at 02:15:52PM +0000, Zack Rusin wrote:
>> On Thu, 2022-04-07 at 08:01 +0200, Christian König wrote:
>>> Am 07.04.22 um 04:56 schrieb Zack Rusin:
>>>> From: Zack Rusin <zackr@vmware.com>
>>>>
>>>> Drivers duplicate the code required to add debugfs entries for
>>>> various
>>>> ttm resource managers. To fix it add common TTM resource manager
>>>> code that each driver can reuse.
>>>>
>>>> Because TTM resource managers can be initialized and set a lot
>>>> later
>>>> than TTM device initialization a seperate init function is
>>>> required.
>>>> Specific resource managers can overwrite
>>>> ttm_resource_manager_func::debug to get more information from those
>>>> debugfs entries.
>>>>
>>>> Signed-off-by: Zack Rusin <zackr@vmware.com>
>>>> Cc: Christian Koenig <christian.koenig@amd.com>
>>>> Cc: Huang Rui <ray.huang@amd.com>
>>>> Cc: David Airlie <airlied@linux.ie>
>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Ah, yes that was on my TODO list for quite a while as well.
>>>
>>>> ---
>>>>    drivers/gpu/drm/ttm/ttm_resource.c | 65
>>>> ++++++++++++++++++++++++++++++
>>>>    include/drm/ttm/ttm_resource.h     |  4 ++
>>>>    2 files changed, 69 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/ttm/ttm_resource.c
>>>> b/drivers/gpu/drm/ttm/ttm_resource.c
>>>> index 492ba3157e75..6392ad3e9a88 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_resource.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_resource.c
>>>> @@ -29,6 +29,8 @@
>>>>    #include <drm/ttm/ttm_resource.h>
>>>>    #include <drm/ttm/ttm_bo_driver.h>
>>>>
>>>> +#include "ttm_module.h"
>>>> +
>>>>    /**
>>>>     * ttm_lru_bulk_move_init - initialize a bulk move structure
>>>>     * @bulk: the structure to init
>>>> @@ -644,3 +646,66 @@ ttm_kmap_iter_linear_io_fini(struct
>>>> ttm_kmap_iter_linear_io *iter_io,
>>>>
>>>>        ttm_mem_io_free(bdev, mem);
>>>>    }
>>>> +
>>>> +#if defined(CONFIG_DEBUG_FS)
>>>> +
>>>> +#define TTM_RES_MAN_SHOW(i) \
>>>> +     static int ttm_resource_manager##i##_show(struct seq_file *m,
>>>> void *unused) \
>>>> +     { \
>>>> +             struct ttm_device *bdev = (struct ttm_device *)m-
>>>>> private; \
>>>> +             struct ttm_resource_manager *man =
>>>> ttm_manager_type(bdev, i); \
>>>> +             struct drm_printer p = drm_seq_file_printer(m); \
>>>> +             ttm_resource_manager_debug(man, &p); \
>>>> +             return 0; \
>>>> +     }\
>>>> +     DEFINE_SHOW_ATTRIBUTE(ttm_resource_manager##i)
>>>> +
>>>> +TTM_RES_MAN_SHOW(0);
>>>> +TTM_RES_MAN_SHOW(1);
>>>> +TTM_RES_MAN_SHOW(2);
>>>> +TTM_RES_MAN_SHOW(3);
>>>> +TTM_RES_MAN_SHOW(4);
>>>> +TTM_RES_MAN_SHOW(5);
>>>> +TTM_RES_MAN_SHOW(6);
>>> Uff, please not a static array.
>>>
>>>> +
>>>> +#endif
>>>> +
>>>> +/**
>>>> + * ttm_resource_manager_debugfs_init - Setup debugfs entries for
>>>> specified
>>>> + * resource managers.
>>>> + * @bdev: The TTM device
>>>> + * @file_names: A mapping between TTM_TT placements and the
>>>> debugfs file
>>>> + * names
>>>> + * @num_file_names: The array size of @file_names.
>>>> + *
>>>> + * This function setups up debugfs files that can be used to look
>>>> + * at debug statistics of the specified ttm_resource_managers.
>>>> + * @file_names array is used to figure out which ttm placements
>>>> + * will get debugfs files created for them.
>>>> + */
>>>> +void
>>>> +ttm_resource_manager_debugfs_init(struct ttm_device *bdev,
>>>> +                               const char * const file_names[],
>>>> +                               uint32_t num_file_names)
>>>> +{
>>>> +#if defined(CONFIG_DEBUG_FS)
>>>> +     uint32_t i;
>>>> +     const struct file_operations *fops[] = {
>>>> +             &ttm_resource_manager0_fops,
>>>> +             &ttm_resource_manager1_fops,
>>>> +             &ttm_resource_manager2_fops,
>>>> +             &ttm_resource_manager3_fops,
>>>> +             &ttm_resource_manager4_fops,
>>>> +             &ttm_resource_manager5_fops,
>>>> +             &ttm_resource_manager6_fops,
>>>> +     };
>>>> +
>>>> +     WARN_ON(num_file_names > ARRAY_SIZE(fops));
>>>> +
>>>> +     for (i = 0; i < num_file_names; ++i)
>>>> +             if (file_names[i] && fops[i])
>>>> +                     debugfs_create_file(file_names[i], 0444,
>>>> +                                         ttm_debugfs_root, bdev,
>>>> fops[i]);
>>> You can give the ttm_resource_manager directly as parameter here
>>> instead
>>> of the bdev and avoid the whole handling with the macro and global
>>> arrays.
>> We could but that does change the behaviour. I was trying to preserve
>> the old semantics. Because the lifetimes of driver specific managers
>> are not handled by ttm, there's nothing preventing the drivers from,
>> e.g. during reset, deleting the old and setting up new resource
>> managers. By always using ttm_manager_type() we make sure that we're
>> always using the current one. Passing ttm_resource_manager directly
>> makes it impossible to validate that at _show() time
>> ttm_resource_manager is still valid. It's not a problem for vmwgfx
>> because we never reset the resource managers but I didn't feel
>> comfortable changing the semantics like that for all drivers. It could
>> lead to weird crashes, but if you prefer that approach I'm happy to
>> change it.
> Uh resetting resource managers during the lifetime of a drm_device sounds
> very broken. I guess it's somewhat ok over suspend/resume or so when
> userspace cannot access the driver, but even then it smells fishy.
>
> Unless we have a driver doing that I don't think this is a use-case we
> should support.

Yes, completely agree.

We often use the placement enum only because it used to be a flag and 
I'm working on to get rid of that and use a pointer to the manager where 
an allocation comes from directly.

Regards,
Christian.

> -Daniel