* [PATCH v2 0/1] drm/panfrost: Replace fdinfo's profiling debugfs knob @ 2024-03-02 15:48 Adrián Larumbe 2024-03-02 15:48 ` [PATCH v2 1/1] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs Adrián Larumbe 0 siblings, 1 reply; 4+ messages in thread From: Adrián Larumbe @ 2024-03-02 15:48 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Steven Price, David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Jonathan Corbet Cc: Adrián Larumbe, linux-kernel, dri-devel, linux-doc This is v2 of the patch already discussed in [1] Changelog: - Turned the profile mode atomic variable into a boolean - Rewrote the sysfs file's uAPI documentation to make it more generic - Improved the casting of the profiling variable inside the Panfrost device structure [1]https://lore.kernel.org/dri-devel/20240221161237.2478193-1-adrian.larumbe@collabora.com/ Adrián Larumbe (1): drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs .../testing/sysfs-driver-panfrost-profiling | 10 +++ Documentation/gpu/panfrost.rst | 9 +++ drivers/gpu/drm/panfrost/Makefile | 5 +- drivers/gpu/drm/panfrost/panfrost_debugfs.c | 21 ------ drivers/gpu/drm/panfrost/panfrost_debugfs.h | 14 ---- drivers/gpu/drm/panfrost/panfrost_device.h | 5 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 14 ++-- drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- drivers/gpu/drm/panfrost/panfrost_sysfs.c | 70 +++++++++++++++++++ drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 ++++ 10 files changed, 120 insertions(+), 45 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-panfrost-profiling delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.c create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.h base-commit: 216c1282dde38ca87ebdf1ccacee5a0682901574 -- 2.43.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs 2024-03-02 15:48 [PATCH v2 0/1] drm/panfrost: Replace fdinfo's profiling debugfs knob Adrián Larumbe @ 2024-03-02 15:48 ` Adrián Larumbe 2024-03-04 16:04 ` Steven Price 0 siblings, 1 reply; 4+ messages in thread From: Adrián Larumbe @ 2024-03-02 15:48 UTC (permalink / raw) To: Boris Brezillon, Rob Herring, Steven Price, David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Jonathan Corbet Cc: Adrián Larumbe, linux-kernel, dri-devel, linux-doc Debugfs isn't always available in production builds that try to squeeze every single byte out of the kernel image, but we still need a way to toggle the timestamp and cycle counter registers so that jobs can be profiled for fdinfo's drm engine and cycle calculations. Drop the debugfs knob and replace it with a sysfs file that accomplishes the same functionality, and document its ABI in a separate file. Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- .../testing/sysfs-driver-panfrost-profiling | 10 +++ Documentation/gpu/panfrost.rst | 9 +++ drivers/gpu/drm/panfrost/Makefile | 5 +- drivers/gpu/drm/panfrost/panfrost_debugfs.c | 21 ------ drivers/gpu/drm/panfrost/panfrost_debugfs.h | 14 ---- drivers/gpu/drm/panfrost/panfrost_device.h | 5 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 14 ++-- drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- drivers/gpu/drm/panfrost/panfrost_sysfs.c | 70 +++++++++++++++++++ drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 ++++ 10 files changed, 120 insertions(+), 45 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-panfrost-profiling delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.c create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.h diff --git a/Documentation/ABI/testing/sysfs-driver-panfrost-profiling b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling new file mode 100644 index 000000000000..889527b71b9d --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling @@ -0,0 +1,10 @@ +What: /sys/bus/.../drivers/panfrost/.../drm/../profiling/status +Date: February 2024 +KernelVersion: 6.8.0 +Contact: Adrian Larumbe <adrian.larumbe@collabora.com> +Description: + Get/set drm fdinfo's engine and cycles profiling status. + Valid values are: + 0: Don't enable fdinfo job profiling sources. + 1: Enable fdinfo job profiling sources, this enables both the GPU's + timestamp and cycle counter registers. \ No newline at end of file diff --git a/Documentation/gpu/panfrost.rst b/Documentation/gpu/panfrost.rst index b80e41f4b2c5..be4ac282ef63 100644 --- a/Documentation/gpu/panfrost.rst +++ b/Documentation/gpu/panfrost.rst @@ -38,3 +38,12 @@ the currently possible format options: Possible `drm-engine-` key names are: `fragment`, and `vertex-tiler`. `drm-curfreq-` values convey the current operating frequency for that engine. + +Users must bear in mind that engine and cycle sampling are disabled by default, +because of power saving concerns. `fdinfo` users and benchmark applications which +query the fdinfo file must make sure to toggle the job profiling status of the +driver by writing into the appropriate sysfs node:: + + echo <N> > /sys/bus/platform/drivers/panfrost/[a-f0-9]*.gpu/drm/card1/profiling + +Where `N` is either `0` or `1`, depending on the desired enablement status. diff --git a/drivers/gpu/drm/panfrost/Makefile b/drivers/gpu/drm/panfrost/Makefile index 2c01c1e7523e..6e718595d8a6 100644 --- a/drivers/gpu/drm/panfrost/Makefile +++ b/drivers/gpu/drm/panfrost/Makefile @@ -10,8 +10,7 @@ panfrost-y := \ panfrost_job.o \ panfrost_mmu.o \ panfrost_perfcnt.o \ - panfrost_dump.o - -panfrost-$(CONFIG_DEBUG_FS) += panfrost_debugfs.o + panfrost_dump.o \ + panfrost_sysfs.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 deleted file mode 100644 index 72d4286a6bf7..000000000000 --- a/drivers/gpu/drm/panfrost/panfrost_debugfs.c +++ /dev/null @@ -1,21 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -/* Copyright 2023 Collabora ltd. */ -/* Copyright 2023 Amazon.com, Inc. or its affiliates. */ - -#include <linux/debugfs.h> -#include <linux/platform_device.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" - -void panfrost_debugfs_init(struct drm_minor *minor) -{ - struct drm_device *dev = minor->dev; - struct panfrost_device *pfdev = platform_get_drvdata(to_platform_device(dev->dev)); - - debugfs_create_atomic_t("profile", 0600, minor->debugfs_root, &pfdev->profile_mode); -} diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.h b/drivers/gpu/drm/panfrost/panfrost_debugfs.h deleted file mode 100644 index c5af5f35877f..000000000000 --- a/drivers/gpu/drm/panfrost/panfrost_debugfs.h +++ /dev/null @@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/* - * Copyright 2023 Collabora ltd. - * Copyright 2023 Amazon.com, Inc. or its affiliates. - */ - -#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_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index 62f7e3527385..2f3580c7ba0d 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -130,7 +130,10 @@ struct panfrost_device { struct list_head scheduled_jobs; struct panfrost_perfcnt *perfcnt; - atomic_t profile_mode; + struct kobj_profiling { + struct kobject base; + bool profile_mode; + } profiling; struct mutex sched_lock; diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index a926d71e8131..6db1ea453514 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -20,7 +20,7 @@ #include "panfrost_job.h" #include "panfrost_gpu.h" #include "panfrost_perfcnt.h" -#include "panfrost_debugfs.h" +#include "panfrost_sysfs.h" static bool unstable_ioctls; module_param_unsafe(unstable_ioctls, bool, 0600); @@ -600,10 +600,6 @@ 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) @@ -663,8 +659,14 @@ static int panfrost_probe(struct platform_device *pdev) if (err) goto err_out2; + err = panfrost_sysfs_init(pfdev); + if (err) + goto err_out3; + return 0; +err_out3: + panfrost_gem_shrinker_cleanup(ddev); err_out2: drm_dev_unregister(ddev); err_out1: @@ -681,6 +683,8 @@ static void panfrost_remove(struct platform_device *pdev) struct panfrost_device *pfdev = platform_get_drvdata(pdev); struct drm_device *ddev = pfdev->ddev; + panfrost_sysfs_cleanup(pfdev); + drm_dev_unregister(ddev); panfrost_gem_shrinker_cleanup(ddev); diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 0c2dbf6ef2a5..1be9c31b2b61 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -243,7 +243,7 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) subslot = panfrost_enqueue_job(pfdev, js, job); /* Don't queue the job if a reset is in progress */ if (!atomic_read(&pfdev->reset.pending)) { - if (atomic_read(&pfdev->profile_mode)) { + if (pfdev->profiling.profile_mode) { panfrost_cycle_counter_get(pfdev); job->is_profiled = true; job->start_time = ktime_get(); diff --git a/drivers/gpu/drm/panfrost/panfrost_sysfs.c b/drivers/gpu/drm/panfrost/panfrost_sysfs.c new file mode 100644 index 000000000000..380d74e61611 --- /dev/null +++ b/drivers/gpu/drm/panfrost/panfrost_sysfs.c @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright 2023 Collabora ltd. */ +/* Copyright 2023 Amazon.com, Inc. or its affiliates. */ + +#include <linux/platform_device.h> +#include <drm/drm_file.h> +#include <drm/panfrost_drm.h> + +#include "panfrost_device.h" +#include "panfrost_gpu.h" +#include "panfrost_sysfs.h" + +static ssize_t +profiling_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) +{ + bool *profile_mode = + &container_of(kobj, struct panfrost_device, + profiling.base)->profiling.profile_mode; + + return sysfs_emit(buf, "%d\n", *profile_mode); +} + +static ssize_t +profiling_store(struct kobject *kobj, struct kobj_attribute *attr, + const char *buf, size_t count) +{ + bool *profile_mode = + &container_of(kobj, struct panfrost_device, + profiling.base)->profiling.profile_mode; + int err, value; + + err = kstrtoint(buf, 0, &value); + if (err) + return err; + + *profile_mode = !!value; + + return count; +} + +static const struct kobj_attribute profiling_status = +__ATTR(status, 0644, profiling_show, profiling_store); + +static const struct kobj_type kobj_profile_type = { + .sysfs_ops = &kobj_sysfs_ops, +}; + +int panfrost_sysfs_init(struct panfrost_device *pfdev) +{ + struct device *kdev = pfdev->ddev->primary->kdev; + int err; + + kobject_init(&pfdev->profiling.base, &kobj_profile_type); + + err = kobject_add(&pfdev->profiling.base, &kdev->kobj, "%s", "profiling"); + if (err) + return err; + + err = sysfs_create_file(&pfdev->profiling.base, &profiling_status.attr); + if (err) + kobject_del(&pfdev->profiling.base); + + return err; +} + +void panfrost_sysfs_cleanup(struct panfrost_device *pfdev) +{ + sysfs_remove_file(&pfdev->profiling.base, &profiling_status.attr); + kobject_del(&pfdev->profiling.base); +} diff --git a/drivers/gpu/drm/panfrost/panfrost_sysfs.h b/drivers/gpu/drm/panfrost/panfrost_sysfs.h new file mode 100644 index 000000000000..5fc9c8c1091a --- /dev/null +++ b/drivers/gpu/drm/panfrost/panfrost_sysfs.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright 2023 Collabora ltd. + * Copyright 2023 Amazon.com, Inc. or its affiliates. + */ + +#ifndef PANFROST_SYSFS_H +#define PANFROST_SYSFS_H + +struct panfrost_device; + +int panfrost_sysfs_init(struct panfrost_device *pfdev); +void panfrost_sysfs_cleanup(struct panfrost_device *pfdev); + +#endif /* PANFROST_SYSFS_H */ -- 2.43.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs 2024-03-02 15:48 ` [PATCH v2 1/1] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs Adrián Larumbe @ 2024-03-04 16:04 ` Steven Price 2024-03-04 17:03 ` Boris Brezillon 0 siblings, 1 reply; 4+ messages in thread From: Steven Price @ 2024-03-04 16:04 UTC (permalink / raw) To: Adrián Larumbe, Boris Brezillon, Rob Herring, David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Jonathan Corbet Cc: linux-kernel, dri-devel, linux-doc On 02/03/2024 15:48, Adrián Larumbe wrote: > Debugfs isn't always available in production builds that try to squeeze > every single byte out of the kernel image, but we still need a way to > toggle the timestamp and cycle counter registers so that jobs can be > profiled for fdinfo's drm engine and cycle calculations. > > Drop the debugfs knob and replace it with a sysfs file that accomplishes > the same functionality, and document its ABI in a separate file. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> I'm happy with this. Reviewed-by: Steven Price <steven.price@arm.com> Boris: are you happy with the sysfs ABI, or would you like to investigate further the implications of leaving the counters enabled all the time during execution before committing to the sysfs ABI? Steve > --- > .../testing/sysfs-driver-panfrost-profiling | 10 +++ > Documentation/gpu/panfrost.rst | 9 +++ > drivers/gpu/drm/panfrost/Makefile | 5 +- > drivers/gpu/drm/panfrost/panfrost_debugfs.c | 21 ------ > drivers/gpu/drm/panfrost/panfrost_debugfs.h | 14 ---- > drivers/gpu/drm/panfrost/panfrost_device.h | 5 +- > drivers/gpu/drm/panfrost/panfrost_drv.c | 14 ++-- > drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- > drivers/gpu/drm/panfrost/panfrost_sysfs.c | 70 +++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 ++++ > 10 files changed, 120 insertions(+), 45 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-driver-panfrost-profiling > delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.c > delete mode 100644 drivers/gpu/drm/panfrost/panfrost_debugfs.h > create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.c > create mode 100644 drivers/gpu/drm/panfrost/panfrost_sysfs.h > > diff --git a/Documentation/ABI/testing/sysfs-driver-panfrost-profiling b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling > new file mode 100644 > index 000000000000..889527b71b9d > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling > @@ -0,0 +1,10 @@ > +What: /sys/bus/.../drivers/panfrost/.../drm/../profiling/status > +Date: February 2024 > +KernelVersion: 6.8.0 > +Contact: Adrian Larumbe <adrian.larumbe@collabora.com> > +Description: > + Get/set drm fdinfo's engine and cycles profiling status. > + Valid values are: > + 0: Don't enable fdinfo job profiling sources. > + 1: Enable fdinfo job profiling sources, this enables both the GPU's > + timestamp and cycle counter registers. > \ No newline at end of file > diff --git a/Documentation/gpu/panfrost.rst b/Documentation/gpu/panfrost.rst > index b80e41f4b2c5..be4ac282ef63 100644 > --- a/Documentation/gpu/panfrost.rst > +++ b/Documentation/gpu/panfrost.rst > @@ -38,3 +38,12 @@ the currently possible format options: > > Possible `drm-engine-` key names are: `fragment`, and `vertex-tiler`. > `drm-curfreq-` values convey the current operating frequency for that engine. > + > +Users must bear in mind that engine and cycle sampling are disabled by default, > +because of power saving concerns. `fdinfo` users and benchmark applications which > +query the fdinfo file must make sure to toggle the job profiling status of the > +driver by writing into the appropriate sysfs node:: > + > + echo <N> > /sys/bus/platform/drivers/panfrost/[a-f0-9]*.gpu/drm/card1/profiling > + > +Where `N` is either `0` or `1`, depending on the desired enablement status. > diff --git a/drivers/gpu/drm/panfrost/Makefile b/drivers/gpu/drm/panfrost/Makefile > index 2c01c1e7523e..6e718595d8a6 100644 > --- a/drivers/gpu/drm/panfrost/Makefile > +++ b/drivers/gpu/drm/panfrost/Makefile > @@ -10,8 +10,7 @@ panfrost-y := \ > panfrost_job.o \ > panfrost_mmu.o \ > panfrost_perfcnt.o \ > - panfrost_dump.o > - > -panfrost-$(CONFIG_DEBUG_FS) += panfrost_debugfs.o > + panfrost_dump.o \ > + panfrost_sysfs.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 > deleted file mode 100644 > index 72d4286a6bf7..000000000000 > --- a/drivers/gpu/drm/panfrost/panfrost_debugfs.c > +++ /dev/null > @@ -1,21 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0 > -/* Copyright 2023 Collabora ltd. */ > -/* Copyright 2023 Amazon.com, Inc. or its affiliates. */ > - > -#include <linux/debugfs.h> > -#include <linux/platform_device.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" > - > -void panfrost_debugfs_init(struct drm_minor *minor) > -{ > - struct drm_device *dev = minor->dev; > - struct panfrost_device *pfdev = platform_get_drvdata(to_platform_device(dev->dev)); > - > - debugfs_create_atomic_t("profile", 0600, minor->debugfs_root, &pfdev->profile_mode); > -} > diff --git a/drivers/gpu/drm/panfrost/panfrost_debugfs.h b/drivers/gpu/drm/panfrost/panfrost_debugfs.h > deleted file mode 100644 > index c5af5f35877f..000000000000 > --- a/drivers/gpu/drm/panfrost/panfrost_debugfs.h > +++ /dev/null > @@ -1,14 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0 */ > -/* > - * Copyright 2023 Collabora ltd. > - * Copyright 2023 Amazon.com, Inc. or its affiliates. > - */ > - > -#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_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index 62f7e3527385..2f3580c7ba0d 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -130,7 +130,10 @@ struct panfrost_device { > struct list_head scheduled_jobs; > > struct panfrost_perfcnt *perfcnt; > - atomic_t profile_mode; > + struct kobj_profiling { > + struct kobject base; > + bool profile_mode; > + } profiling; > > struct mutex sched_lock; > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index a926d71e8131..6db1ea453514 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -20,7 +20,7 @@ > #include "panfrost_job.h" > #include "panfrost_gpu.h" > #include "panfrost_perfcnt.h" > -#include "panfrost_debugfs.h" > +#include "panfrost_sysfs.h" > > static bool unstable_ioctls; > module_param_unsafe(unstable_ioctls, bool, 0600); > @@ -600,10 +600,6 @@ 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) > @@ -663,8 +659,14 @@ static int panfrost_probe(struct platform_device *pdev) > if (err) > goto err_out2; > > + err = panfrost_sysfs_init(pfdev); > + if (err) > + goto err_out3; > + > return 0; > > +err_out3: > + panfrost_gem_shrinker_cleanup(ddev); > err_out2: > drm_dev_unregister(ddev); > err_out1: > @@ -681,6 +683,8 @@ static void panfrost_remove(struct platform_device *pdev) > struct panfrost_device *pfdev = platform_get_drvdata(pdev); > struct drm_device *ddev = pfdev->ddev; > > + panfrost_sysfs_cleanup(pfdev); > + > drm_dev_unregister(ddev); > panfrost_gem_shrinker_cleanup(ddev); > > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > index 0c2dbf6ef2a5..1be9c31b2b61 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -243,7 +243,7 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) > subslot = panfrost_enqueue_job(pfdev, js, job); > /* Don't queue the job if a reset is in progress */ > if (!atomic_read(&pfdev->reset.pending)) { > - if (atomic_read(&pfdev->profile_mode)) { > + if (pfdev->profiling.profile_mode) { > panfrost_cycle_counter_get(pfdev); > job->is_profiled = true; > job->start_time = ktime_get(); > diff --git a/drivers/gpu/drm/panfrost/panfrost_sysfs.c b/drivers/gpu/drm/panfrost/panfrost_sysfs.c > new file mode 100644 > index 000000000000..380d74e61611 > --- /dev/null > +++ b/drivers/gpu/drm/panfrost/panfrost_sysfs.c > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright 2023 Collabora ltd. */ > +/* Copyright 2023 Amazon.com, Inc. or its affiliates. */ > + > +#include <linux/platform_device.h> > +#include <drm/drm_file.h> > +#include <drm/panfrost_drm.h> > + > +#include "panfrost_device.h" > +#include "panfrost_gpu.h" > +#include "panfrost_sysfs.h" > + > +static ssize_t > +profiling_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) > +{ > + bool *profile_mode = > + &container_of(kobj, struct panfrost_device, > + profiling.base)->profiling.profile_mode; > + > + return sysfs_emit(buf, "%d\n", *profile_mode); > +} > + > +static ssize_t > +profiling_store(struct kobject *kobj, struct kobj_attribute *attr, > + const char *buf, size_t count) > +{ > + bool *profile_mode = > + &container_of(kobj, struct panfrost_device, > + profiling.base)->profiling.profile_mode; > + int err, value; > + > + err = kstrtoint(buf, 0, &value); > + if (err) > + return err; > + > + *profile_mode = !!value; > + > + return count; > +} > + > +static const struct kobj_attribute profiling_status = > +__ATTR(status, 0644, profiling_show, profiling_store); > + > +static const struct kobj_type kobj_profile_type = { > + .sysfs_ops = &kobj_sysfs_ops, > +}; > + > +int panfrost_sysfs_init(struct panfrost_device *pfdev) > +{ > + struct device *kdev = pfdev->ddev->primary->kdev; > + int err; > + > + kobject_init(&pfdev->profiling.base, &kobj_profile_type); > + > + err = kobject_add(&pfdev->profiling.base, &kdev->kobj, "%s", "profiling"); > + if (err) > + return err; > + > + err = sysfs_create_file(&pfdev->profiling.base, &profiling_status.attr); > + if (err) > + kobject_del(&pfdev->profiling.base); > + > + return err; > +} > + > +void panfrost_sysfs_cleanup(struct panfrost_device *pfdev) > +{ > + sysfs_remove_file(&pfdev->profiling.base, &profiling_status.attr); > + kobject_del(&pfdev->profiling.base); > +} > diff --git a/drivers/gpu/drm/panfrost/panfrost_sysfs.h b/drivers/gpu/drm/panfrost/panfrost_sysfs.h > new file mode 100644 > index 000000000000..5fc9c8c1091a > --- /dev/null > +++ b/drivers/gpu/drm/panfrost/panfrost_sysfs.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright 2023 Collabora ltd. > + * Copyright 2023 Amazon.com, Inc. or its affiliates. > + */ > + > +#ifndef PANFROST_SYSFS_H > +#define PANFROST_SYSFS_H > + > +struct panfrost_device; > + > +int panfrost_sysfs_init(struct panfrost_device *pfdev); > +void panfrost_sysfs_cleanup(struct panfrost_device *pfdev); > + > +#endif /* PANFROST_SYSFS_H */ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs 2024-03-04 16:04 ` Steven Price @ 2024-03-04 17:03 ` Boris Brezillon 0 siblings, 0 replies; 4+ messages in thread From: Boris Brezillon @ 2024-03-04 17:03 UTC (permalink / raw) To: Steven Price Cc: Adrián Larumbe, Rob Herring, David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Jonathan Corbet, linux-kernel, dri-devel, linux-doc On Mon, 4 Mar 2024 16:04:34 +0000 Steven Price <steven.price@arm.com> wrote: > On 02/03/2024 15:48, Adrián Larumbe wrote: > > Debugfs isn't always available in production builds that try to squeeze > > every single byte out of the kernel image, but we still need a way to > > toggle the timestamp and cycle counter registers so that jobs can be > > profiled for fdinfo's drm engine and cycle calculations. > > > > Drop the debugfs knob and replace it with a sysfs file that accomplishes > > the same functionality, and document its ABI in a separate file. > > > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > > I'm happy with this. > > Reviewed-by: Steven Price <steven.price@arm.com> > > Boris: are you happy with the sysfs ABI, or would you like to > investigate further the implications of leaving the counters enabled all > the time during execution before committing to the sysfs ABI? No, that's fine, but I have a few comments on the implementation. > > +static ssize_t > > +profiling_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) > > +{ > > + bool *profile_mode = > > + &container_of(kobj, struct panfrost_device, > > + profiling.base)->profiling.profile_mode; > > + > > + return sysfs_emit(buf, "%d\n", *profile_mode); > > +} > > + > > +static ssize_t > > +profiling_store(struct kobject *kobj, struct kobj_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + bool *profile_mode = > > + &container_of(kobj, struct panfrost_device, > > + profiling.base)->profiling.profile_mode; > > + int err, value; > > + > > + err = kstrtoint(buf, 0, &value); I'd suggest using kstrtobool() since you make the result a boolean anyway. > > + if (err) > > + return err; > > + > > + *profile_mode = !!value; > > + > > + return count; > > +} > > + > > +static const struct kobj_attribute profiling_status = > > +__ATTR(status, 0644, profiling_show, profiling_store); > > + > > +static const struct kobj_type kobj_profile_type = { > > + .sysfs_ops = &kobj_sysfs_ops, > > +}; DEVICE_ATTR(profiling, 0644, profiling_show, profiling_store); ? > > + > > +int panfrost_sysfs_init(struct panfrost_device *pfdev) > > +{ > > + struct device *kdev = pfdev->ddev->primary->kdev; > > + int err; > > + > > + kobject_init(&pfdev->profiling.base, &kobj_profile_type); > > + > > + err = kobject_add(&pfdev->profiling.base, &kdev->kobj, "%s", "profiling"); Can we make it a device attribute instead of adding an extra kboj? > > + if (err) > > + return err; > > + > > + err = sysfs_create_file(&pfdev->profiling.base, &profiling_status.attr); > > + if (err) > > + kobject_del(&pfdev->profiling.base); > > + > > + return err; > > +} ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-04 17:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-02 15:48 [PATCH v2 0/1] drm/panfrost: Replace fdinfo's profiling debugfs knob Adrián Larumbe 2024-03-02 15:48 ` [PATCH v2 1/1] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs Adrián Larumbe 2024-03-04 16:04 ` Steven Price 2024-03-04 17:03 ` Boris Brezillon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®