Re: [Intel-gfx] [PATCH 5/6] drm/i915/uc: Move uC debugfs to its own folder under GT

[Andi Shyti <andi.shyti@intel.com>]

Hi Daniele,

I'm sorry I missed this patch,

On Thu, Feb 27, 2020 at 06:28:42PM -0800, Daniele Ceraolo Spurio wrote:
> uC is a component of the GT, so it makes sense for the uC debugfs files
> to be in the GT folder. A subfolder has been used to keep the same
> structure we have for the code.

Can we please document the interface changes. I see there are
some differences between the original and the new interfaces.

> +#define DEFINE_UC_DEBUGFS_ATTRIBUTE(__name)				\
> +	static int __name ## _open(struct inode *inode, struct file *file) \
> +{									\
> +	return single_open(file, __name ## _show, inode->i_private);	\
> +}									\
> +static const struct file_operations __name ## _fops = {			\
> +	.owner = THIS_MODULE,						\
> +	.open = __name ## _open,					\
> +	.read = seq_read,						\
> +	.llseek = seq_lseek,						\
> +	.release = single_release,					\
> +}

Why do we need DEFINE_UC_DEBUGFS_ATTRIBUTE()?

DEFINE_GT_DEBUGFS_ATTRIBUTE() was meant to be common to all gt
debugfs. I there any reason we need a new one?

> +struct debugfs_uc_file {
> +	const char *name;
> +	const struct file_operations *fops;
> +};
> +
> +#define debugfs_uc_register_files(files__, root__, data__) \
> +do { \
> +	int i__ = 0; \
> +	for (i__ = 0; i__ < ARRAY_SIZE(files__); i__++) { \
> +		debugfs_create_file(files__[i__].name, \
> +				    0444, root__, data__, \
> +				    files__[i__].fops); \
> +	} \
> +} while (0)

You want to define your own debugfs_uc_register_files() instead
of using debugfs_gt_register_files() because you want "data__"
to be void, right?

I think we can achieve that by adding a wrapper in debugfs_gt.c,
perhaps we can do something like:

void __debugfs_gt_register_files(struct intel_gt *gt,
                                 struct dentry *root,
                                 const struct debugfs_gt_file *files,
                                 void *data,
                                 unsigned long count)
{
      ......
}

and 

#define debugfs_gt_register_files(...) __debugfs_gt_register_files(...)
#define debugfs_uc_register_files(...) __debugfs_gt_register_files(...)

so that we can keep everything in a library. What do you think.

Thanks,
Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx