Message ID | 20230905184533.959171-4-adrian.larumbe@collabora.com |
---|---|
State | New |
Headers | show |
Series | Add fdinfo support to Panfrost | expand |
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)
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); + 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); +} 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)
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