[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 at 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 at vmware.com>
> Cc: Christian Koenig <christian.koenig at amd.com>
> Cc: Huang Rui <ray.huang at amd.com>
> Cc: David Airlie <airlied at linux.ie>
> Cc: Daniel Vetter <daniel at 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