[v3,3/8] drm/panfrost: Enable debugfs toggling of cycle counter register

Allow user space to decide whether the cycle counting register should be
enabled. The main goal is letting tools like nvtop or IGT's gputop access
this information in debug builds to obtain engine utilisation numbers.

Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
 drivers/gpu/drm/panfrost/Makefile           |  2 +
 drivers/gpu/drm/panfrost/panfrost_debugfs.c | 51 +++++++++++++++++++++
 drivers/gpu/drm/panfrost/panfrost_debugfs.h | 13 ++++++
 drivers/gpu/drm/panfrost/panfrost_drv.c     |  5 ++
 4 files changed, 71 insertions(+)
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c
 create mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h

On Tue,  5 Sep 2023 19:45:19 +0100
Adrián Larumbe <adrian.larumbe@collabora.com> wrote:

> Allow user space to decide whether the cycle counting register should be
> enabled. The main goal is letting tools like nvtop or IGT's gputop access
> this information in debug builds to obtain engine utilisation numbers.

Given you add a debugfs knob, I would simply drop the module param
introduced in the previous patch, it's just redundant, and I'd expect
people who care about profiling to have DEBUGFS enabled anyway.

> 
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
>  drivers/gpu/drm/panfrost/Makefile           |  2 +
>  drivers/gpu/drm/panfrost/panfrost_debugfs.c | 51 +++++++++++++++++++++
>  drivers/gpu/drm/panfrost/panfrost_debugfs.h | 13 ++++++
>  drivers/gpu/drm/panfrost/panfrost_drv.c     |  5 ++
>  4 files changed, 71 insertions(+)
>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c
>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h
> 
> diff --git a/drivers/gpu/drm/panfrost/Makefile b/drivers/gpu/drm/panfrost/Makefile
> index 7da2b3f02ed9..2c01c1e7523e 100644
> --- a/drivers/gpu/drm/panfrost/Makefile
> +++ b/drivers/gpu/drm/panfrost/Makefile
> @@ -12,4 +12,6 @@ panfrost-y := \
>  	panfrost_perfcnt.o \
>  	panfrost_dump.o
>  
> +panfrost-$(CONFIG_DEBUG_FS) += panfrost_debugfs.o
> +
>  obj-$(CONFIG_DRM_PANFROST) += panfrost.o
> diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.c b/drivers/gpu/drm/panfrost/panfrost_debugfs.c
> new file mode 100644
> index 000000000000..48d5ddfcb1c6
> --- /dev/null
> +++ b/drivers/gpu/drm/panfrost/panfrost_debugfs.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright 2023 Collabora ltd. */
> +
> +#include <linux/debugfs.h>
> +#include <drm/drm_debugfs.h>
> +#include <drm/drm_file.h>
> +#include <drm/panfrost_drm.h>
> +
> +#include "panfrost_device.h"
> +#include "panfrost_gpu.h"
> +#include "panfrost_debugfs.h"
> +
> +static int
> +profile_get(void *data, u64 *val)
> +{
> +	struct drm_device *dev = data;
> +	struct panfrost_device *pfdev = dev->dev_private;
> +
> +	*val = atomic_read(&pfdev->profile_mode);
> +
> +	return 0;
> +}
> +
> +static int
> +profile_set(void *data, u64 val)
> +{
> +	struct drm_device *dev = data;
> +	struct panfrost_device *pfdev = dev->dev_private;
> +
> +	if (atomic_read(&pfdev->profile_mode) == val)
> +		return 0;
> +
> +	if (val == false)
> +		panfrost_cycle_counter_stop(pfdev);

I don't think we want to stop the counter immediately. We should
instead let the job logic switch it off when all jobs with profiling
enabled are done.

> +	else
> +		atomic_set(&pfdev->profile_mode, 1);
> +
> +	return 0;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(profile_fops,
> +			 profile_get, profile_set,
> +			 "%llu\n");
> +
> +void panfrost_debugfs_init(struct drm_minor *minor)
> +{
> +	struct drm_device *dev = minor->dev;
> +
> +	debugfs_create_file("profile", 0600, minor->debugfs_root,
> +		dev, &profile_fops);

With the panfrost_cycle_counter_stop() call in profile_set() dropped,
you should be able to use debugfs_create_atomic_t() here.

> +}
> diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.h b/drivers/gpu/drm/panfrost/panfrost_debugfs.h
> new file mode 100644
> index 000000000000..db1c158bcf2f
> --- /dev/null
> +++ b/drivers/gpu/drm/panfrost/panfrost_debugfs.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright 2023 Collabora ltd.
> + */
> +
> +#ifndef PANFROST_DEBUGFS_H
> +#define PANFROST_DEBUGFS_H
> +
> +#ifdef CONFIG_DEBUG_FS
> +void panfrost_debugfs_init(struct drm_minor *minor);
> +#endif
> +
> +#endif  /* PANFROST_DEBUGFS_H */
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index a2ab99698ca8..2dfd9f79a31b 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -20,6 +20,7 @@
>  #include "panfrost_job.h"
>  #include "panfrost_gpu.h"
>  #include "panfrost_perfcnt.h"
> +#include "panfrost_debugfs.h"
>  
>  static bool unstable_ioctls;
>  module_param_unsafe(unstable_ioctls, bool, 0600);
> @@ -546,6 +547,10 @@ static const struct drm_driver panfrost_drm_driver = {
>  
>  	.gem_create_object	= panfrost_gem_create_object,
>  	.gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table,
> +
> +#ifdef CONFIG_DEBUG_FS
> +	.debugfs_init		= panfrost_debugfs_init,
> +#endif
>  };
>  
>  static int panfrost_probe(struct platform_device *pdev)