On 11.03.2024 11:02, Boris Brezillon wrote: > On Wed, 6 Mar 2024 08:33:47 +0000 > Tvrtko Ursulin <tursulin@xxxxxxxxxxx> wrote: > > > On 06/03/2024 01:56, 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@xxxxxxxxxxxxx> > > > --- > > > .../testing/sysfs-driver-panfrost-profiling | 10 +++++ > > > Documentation/gpu/panfrost.rst | 9 ++++ > > > drivers/gpu/drm/panfrost/Makefile | 2 - > > > drivers/gpu/drm/panfrost/panfrost_debugfs.c | 21 ---------- > > > drivers/gpu/drm/panfrost/panfrost_debugfs.h | 14 ------- > > > drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- > > > drivers/gpu/drm/panfrost/panfrost_drv.c | 41 ++++++++++++++++--- > > > drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- > > > 8 files changed, 57 insertions(+), 44 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 > > > > > > diff --git a/Documentation/ABI/testing/sysfs-driver-panfrost-profiling b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling > > > new file mode 100644 > > > index 000000000000..1d8bb0978920 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling > > > @@ -0,0 +1,10 @@ > > > +What: /sys/bus/platform/drivers/panfrost/.../profiling > > > +Date: February 2024 > > > +KernelVersion: 6.8.0 > > > +Contact: Adrian Larumbe <adrian.larumbe@xxxxxxxxxxxxx> > > > +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..51ba375fd80d 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/profiling > > > > A late thought - how it would work to not output the inactive fdinfo > > keys when this knob is not enabled? > > > > Generic userspace like gputop already handles that and wouldn't show the > > stat. Which may be more user friendly than showing stats permanently at > > zero. It may be moot once you add the auto-toggle to gputop (or so) but > > perhaps worth considering. > > I agree with Tvrtko, if the line being printed in fdinfo relies on some > sysfs knob to be valid, we'd rather not print the information in that > case, instead of printing zero. Me too. I'll go first change both gputop and nvtop to make sure they use the new sysfs knob for Panfrost, and then submit a new patch that handles printing of the drm-cycles-* and drm-engine-* stats depending on the profiling knob state.
