Message ID | 20240221161237.2478193-1-adrian.larumbe@collabora.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-75079-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:693c:2685:b0:108:e6aa:91d0 with SMTP id mn5csp1138078dyc; Wed, 21 Feb 2024 08:14:20 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCU5lyxoExRTxMeLqB0n2bWiTliOzbCyHmgUqAjn9LBQD32h2ze3QBLR+lvF+Fxyuqs22SET8/autcH8hVUfIiO+MWhqeA== X-Google-Smtp-Source: AGHT+IFj7AuAq7G5s/vKdkbENkWF8KBbSw7q2O6nB02qNcNizYKlxmscPr5q2DHN9R3z1bjkai8c X-Received: by 2002:a05:6358:5284:b0:178:618b:89bd with SMTP id g4-20020a056358528400b00178618b89bdmr21698661rwa.30.1708532059819; Wed, 21 Feb 2024 08:14:19 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1708532059; cv=pass; d=google.com; s=arc-20160816; b=rpQLwGGrzD4wCgTFjYOblg4YmL4vr0/Dq4b8zajXX+oAQhVEwWfKTRrgJZOsrJmZ5F zF4h4oTlQeMqeGQEFl0WoKRtuQ+Ihe6sYLkB4k2iarC520mTt3Nmf5n+DAOlAKVA0JHN MH2qebzi7lUojY+mg9/a3jeeXfLw7MbGphf/eaWG0OjrCzI7xkZkCwuXFZ3CSQqZMXnB dUh030WUPLf2MnPDGQGQd9eRphocfzxadMofsFf0+zh92ATQEQ4Ff49zdhDB6Es+Tz+C BOeAq13B4/Q4KrwIWTu1eqMSe5dLzx47EUwODqW2+FXd6S0QKiV/HF7b9cRL12nzmvoO AQVw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=xISv0/O3AzmiX6pFzmQAk1IjW9kCI58sUtUZpJmhcv0=; fh=MsrzwZA1LPyGXhLuy1DAxF8H8u3saAN44+gYj3sRUIw=; b=0MRjpRAKbeTvzBvSKirR5r1Ci2vAcnOzgX9ykpaYeyV6LEOTAbWWskFyrXq7Ezd3gU OsVUk7QCpi41o2FVlbxR2kOL24JGPa6PHmQ7CQKJnwlAVlpHFbMMR1bmZXutCOKRcbIi 7rynTZTcph0huOOrfdxa3qNZ7TPLA7IDkrAiS+xMCCQAUd5VqshKG+5epRt1lYLjMNJn CHwqvWBVfTRWhfHoWwlaOm8No/LDfLHmXIaqLuMqOQgajXjcp5Tdv+Yr86Lv1jYU/Qml JpjfPsVYhNnXqrkbD4LQsZx7qWPbn1ee13squDo1NZjkQUBIct2CADCUzRO+8yuQXNOY 01Cg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=QyEFEWg4; arc=pass (i=1 spf=pass spfdomain=collabora.com dkim=pass dkdomain=collabora.com dmarc=pass fromdomain=collabora.com); spf=pass (google.com: domain of linux-kernel+bounces-75079-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-75079-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id e28-20020a63371c000000b005e428bef548si3008387pga.649.2024.02.21.08.14.19 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Feb 2024 08:14:19 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-75079-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=QyEFEWg4; arc=pass (i=1 spf=pass spfdomain=collabora.com dkim=pass dkdomain=collabora.com dmarc=pass fromdomain=collabora.com); spf=pass (google.com: domain of linux-kernel+bounces-75079-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-75079-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 40D04286784 for <ouuuleilei@gmail.com>; Wed, 21 Feb 2024 16:14:19 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 0DFD181741; Wed, 21 Feb 2024 16:14:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="QyEFEWg4" Received: from madrid.collaboradmins.com (madrid.collaboradmins.com [46.235.227.194]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6D77D6994A; Wed, 21 Feb 2024 16:14:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=46.235.227.194 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708532047; cv=none; b=OlpKm+uKT3/y9BsptAsIRu77k+Bg2svvZugG8w+MjZ6Hkylm3HssESDXiYxuY9OZ5HqnCtgv7o3ZJpV0rWyhBX7zWWpUCRtZ1qrzHvyumdWB2w/ahjLyUOjAavvsro0FNZW5VZ6KSoplBz7i1IsF4aFvrhUQQ/BGj8NOVoNlZrQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708532047; c=relaxed/simple; bh=JHBed4HhRS/+miDeC1Vba8uHd/woC+WKGVjOOcm9Eoc=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type; b=OxhT/oiaCH0TW2Yzo0cq447gTi0vhWztuCqZZxuMwg6SoDksRgfZ69alz1uEC0Z3nCC7jMPC0f30YpofiUWwVnN0lJq6P5yOvf667nZSmEkGV+gFmE52cyIu0qexG7YGDRw8pthmIsBc2RYd6TiBQRIdDkxWNcAP5J+KOdkIKRQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=QyEFEWg4; arc=none smtp.client-ip=46.235.227.194 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1708532042; bh=JHBed4HhRS/+miDeC1Vba8uHd/woC+WKGVjOOcm9Eoc=; h=From:To:Cc:Subject:Date:From; b=QyEFEWg4iOj696uNgzJEjM7VVBBxs5TR8MhbBa8yDqmZLFylpV17ZC0v+v3V1T7eL pP9udLVigJOnISA+mi+rJ4g8a+h5shEq/PPyk46PGcQ0Md91kcwNlstwT6y0Rg+ObK BLIlnYBBWgEsHvj7P3H1+88kWgNH1/234/bRn+hxbMm89EtVU7tnkXDn6w9flCNiQ3 PnnxudmewEbTwgpv8sghHFxNCpba4YwuosgP666VBzEWGNhMHaT0efTJn4vzRMIVwp EM1loMtnW8am0K6rs3rzjdzSTAcmVnwwiP+ut2tL412oBvJqzwXypJWOaMf7XSMtxR GbBeI+CJg8v2w== Received: from localhost.localdomain (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: alarumbe) by madrid.collaboradmins.com (Postfix) with ESMTPSA id 9FB2F3781FE6; Wed, 21 Feb 2024 16:14:01 +0000 (UTC) From: =?utf-8?q?Adri=C3=A1n_Larumbe?= <adrian.larumbe@collabora.com> To: Boris Brezillon <boris.brezillon@collabora.com>, Rob Herring <robh@kernel.org>, Steven Price <steven.price@arm.com>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>, Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>, David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, Jonathan Corbet <corbet@lwn.net> Cc: kernel@collabora.com, =?utf-8?q?Adri=C3=A1n_Larumbe?= <adrian.larumbe@collabora.com>, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-doc@vger.kernel.org Subject: [PATCH] drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs Date: Wed, 21 Feb 2024 16:12:32 +0000 Message-ID: <20240221161237.2478193-1-adrian.larumbe@collabora.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1791525712998438999 X-GMAIL-MSGID: 1791525712998438999 |
Series |
drm/panfrost: Replace fdinfo's profiling debugfs knob with sysfs
|
|
Commit Message
Adrián Larumbe
Feb. 21, 2024, 4:12 p.m. UTC
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 | 74 +++++++++++++++++++
drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 ++++
10 files changed, 124 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
Comments
On 21/02/2024 16:12, 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> > --- > .../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 | 74 +++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 ++++ > 10 files changed, 124 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..ce54069714f3 > --- /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: Disable fdinfo job profiling sources. This disables both the GPU's > + timestamp and cycle counter registers. > + 1: Enable the above. Minor point, but if we're going to eventually come up with a generic way of doing this, then we're going to have to think about backwards compatibility for this sysfs file. I would expect in this new world '0' would mean "default behaviour; off unless the new-fangled thing enables profiling" and '1' means "force on". In which case perhaps wording like the below would be clearer: 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. Or am I being too picky over the wording ;) One other small issue below... > 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..56c8e5551335 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; > + atomic_t 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..49413dfda2ea 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 (atomic_read(&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..072d3bf349d2 > --- /dev/null > +++ b/drivers/gpu/drm/panfrost/panfrost_sysfs.c > @@ -0,0 +1,74 @@ > +// 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) > +{ > + atomic_t *profile_mode = > + &container_of(kobj, struct { > + struct kobject base; > + atomic_t profile_mode; }, > + base)->profile_mode; Yuck! ;) That's likely to catch people out if they modify the struct in the header file. Why not "struct kobj_profiling" rather than that inline struct. I'd also argue that kobj_profiling isn't a great name and I'd suggest something with 'panfrost' in the name. Or you could just use the parent struct panfrost_device and not bother naming this sub-struct, e.g. &container_of(kobj, struct panfrost_device, profiling.base)->profiling.profile_mode; Thanks, Steve > + > + return sysfs_emit(buf, "%d\n", atomic_read(profile_mode)); > +} > + > +static ssize_t > +profiling_store(struct kobject *kobj, struct kobj_attribute *attr, > + const char *buf, size_t count) > +{ > + atomic_t *profile_mode = > + &container_of(kobj, struct { > + struct kobject base; > + atomic_t profile_mode; }, > + base)->profile_mode; > + int err, value; > + > + err = kstrtoint(buf, 0, &value); > + if (err) > + return err; > + > + atomic_set(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 */
On Wed, 21 Feb 2024 16:12:32 +0000 Adrián Larumbe <adrian.larumbe@collabora.com> 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> > --- > .../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 | 74 +++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 ++++ > 10 files changed, 124 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..ce54069714f3 > --- /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: Disable fdinfo job profiling sources. This disables both the GPU's > + timestamp and cycle counter registers. > + 1: Enable the above. > 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..56c8e5551335 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; > + atomic_t 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..49413dfda2ea 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 (atomic_read(&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..072d3bf349d2 > --- /dev/null > +++ b/drivers/gpu/drm/panfrost/panfrost_sysfs.c > @@ -0,0 +1,74 @@ > +// 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) > +{ > + atomic_t *profile_mode = > + &container_of(kobj, struct { > + struct kobject base; > + atomic_t profile_mode; }, > + base)->profile_mode; > + > + return sysfs_emit(buf, "%d\n", atomic_read(profile_mode)); > +} > + > +static ssize_t > +profiling_store(struct kobject *kobj, struct kobj_attribute *attr, > + const char *buf, size_t count) > +{ > + atomic_t *profile_mode = > + &container_of(kobj, struct { > + struct kobject base; > + atomic_t profile_mode; }, > + base)->profile_mode; > + int err, value; Can't we just make this a RW module_param to simplify things? I fail to see why we need an atomic here since active cycle-count users are already tracked with an atomic_t that's incremented/decremented everytime a job is submitted/considered done. > + > + err = kstrtoint(buf, 0, &value); > + if (err) > + return err; > + > + atomic_set(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 */
Hi Steve, On 21.02.2024 16:52, Steven Price wrote: > On 21/02/2024 16:12, 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> > > --- > > .../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 | 74 +++++++++++++++++++ > > drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 ++++ > > 10 files changed, 124 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..ce54069714f3 > > --- /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: Disable fdinfo job profiling sources. This disables both the GPU's > > + timestamp and cycle counter registers. > > + 1: Enable the above. > > Minor point, but if we're going to eventually come up with a generic way > of doing this, then we're going to have to think about backwards > compatibility for this sysfs file. I would expect in this new world '0' > would mean "default behaviour; off unless the new-fangled thing enables > profiling" and '1' means "force on". > > In which case perhaps wording like the below would be clearer: > > 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. > > Or am I being too picky over the wording ;) I'm alright with this kind of wording, to keep things as generic as possible. Initially I thought just mentioning 0 and 1 as potential toggle values would be enough, and then every driver could describe their own profiling/status sysfs knob in similar terms, depending on what profiling resouces they act upon. > One other small issue below... > > > 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..56c8e5551335 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; > > + atomic_t 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..49413dfda2ea 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 (atomic_read(&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..072d3bf349d2 > > --- /dev/null > > +++ b/drivers/gpu/drm/panfrost/panfrost_sysfs.c > > @@ -0,0 +1,74 @@ > > +// 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) > > +{ > > + atomic_t *profile_mode = > > + &container_of(kobj, struct { > > + struct kobject base; > > + atomic_t profile_mode; }, > > + base)->profile_mode; > > Yuck! ;) That's likely to catch people out if they modify the struct in > the header file. Sorry about this abomination. I think it slipped through the cracks after a first patch draft in which I didn't want to make the kobject-holding structure a global declaration. > Why not "struct kobj_profiling" rather than that inline struct. I'd also > argue that kobj_profiling isn't a great name and I'd suggest something > with 'panfrost' in the name. Or you could just use the parent struct > panfrost_device and not bother naming this sub-struct, e.g. > > &container_of(kobj, struct panfrost_device, > profiling.base)->profiling.profile_mode; This is a much better way of doing things than mine. Thanks! > Thanks, > Steve > > > + > > + return sysfs_emit(buf, "%d\n", atomic_read(profile_mode)); > > +} > > + > > +static ssize_t > > +profiling_store(struct kobject *kobj, struct kobj_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + atomic_t *profile_mode = > > + &container_of(kobj, struct { > > + struct kobject base; > > + atomic_t profile_mode; }, > > + base)->profile_mode; > > + int err, value; > > + > > + err = kstrtoint(buf, 0, &value); > > + if (err) > > + return err; > > + > > + atomic_set(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 */
Hi Boris, On 26.02.2024 09:51, Boris Brezillon wrote: > On Wed, 21 Feb 2024 16:12:32 +0000 > Adrián Larumbe <adrian.larumbe@collabora.com> 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> > > --- > > .../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 | 74 +++++++++++++++++++ > > drivers/gpu/drm/panfrost/panfrost_sysfs.h | 15 ++++ > > 10 files changed, 124 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..ce54069714f3 > > --- /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: Disable fdinfo job profiling sources. This disables both the GPU's > > + timestamp and cycle counter registers. > > + 1: Enable the above. > > 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..56c8e5551335 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; > > + atomic_t 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..49413dfda2ea 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 (atomic_read(&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..072d3bf349d2 > > --- /dev/null > > +++ b/drivers/gpu/drm/panfrost/panfrost_sysfs.c > > @@ -0,0 +1,74 @@ > > +// 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) > > +{ > > + atomic_t *profile_mode = > > + &container_of(kobj, struct { > > + struct kobject base; > > + atomic_t profile_mode; }, > > + base)->profile_mode; > > + > > + return sysfs_emit(buf, "%d\n", atomic_read(profile_mode)); > > +} > > + > > +static ssize_t > > +profiling_store(struct kobject *kobj, struct kobj_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + atomic_t *profile_mode = > > + &container_of(kobj, struct { > > + struct kobject base; > > + atomic_t profile_mode; }, > > + base)->profile_mode; > > + int err, value; > > Can't we just make this a RW module_param to simplify things? I fail to > see why we need an atomic here since active cycle-count users are > already tracked with an atomic_t that's incremented/decremented > everytime a job is submitted/considered done. I thought about this, and it would greatly simplify this patch to the point that I could get rid of all the sysfs cruft, but then I remembered about someone at Collabora once told me of a very specific customer SoC configuration with two Mali GPU devices. I know this might be very unusual, but I guess they could be interested in profiling a single GPU at a time, as unplausible as it sounds. Regarding the atomic variable for keeping track of the profiling status, your'e right, it's an overkill. I'll just get rid of it altogether in v2. > > + > > + err = kstrtoint(buf, 0, &value); > > + if (err) > > + return err; > > + > > + atomic_set(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 */
diff --git a/Documentation/ABI/testing/sysfs-driver-panfrost-profiling b/Documentation/ABI/testing/sysfs-driver-panfrost-profiling new file mode 100644 index 000000000000..ce54069714f3 --- /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: Disable fdinfo job profiling sources. This disables both the GPU's + timestamp and cycle counter registers. + 1: Enable the above. 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..56c8e5551335 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; + atomic_t 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..49413dfda2ea 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 (atomic_read(&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..072d3bf349d2 --- /dev/null +++ b/drivers/gpu/drm/panfrost/panfrost_sysfs.c @@ -0,0 +1,74 @@ +// 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) +{ + atomic_t *profile_mode = + &container_of(kobj, struct { + struct kobject base; + atomic_t profile_mode; }, + base)->profile_mode; + + return sysfs_emit(buf, "%d\n", atomic_read(profile_mode)); +} + +static ssize_t +profiling_store(struct kobject *kobj, struct kobj_attribute *attr, + const char *buf, size_t count) +{ + atomic_t *profile_mode = + &container_of(kobj, struct { + struct kobject base; + atomic_t profile_mode; }, + base)->profile_mode; + int err, value; + + err = kstrtoint(buf, 0, &value); + if (err) + return err; + + atomic_set(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 */