From: Boris Brezillon <boris.brezillon@collabora.com>
To: "Adrián Larumbe" <adrian.larumbe@collabora.com>
Cc: linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
kernel@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>,
"Simona Vetter" <simona@ffwll.ch>,
"Sumit Semwal" <sumit.semwal@linaro.org>,
"Christian König" <christian.koenig@amd.com>,
linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org
Subject: Re: [PATCH 3/3] drm/panfrost: show device-wide list of DRM GEM objects over DebugFS
Date: Tue, 6 May 2025 09:04:18 +0200 [thread overview]
Message-ID: <20250506090418.3c8242b0@collabora.com> (raw)
In-Reply-To: <20250424022138.709303-4-adrian.larumbe@collabora.com>
On Thu, 24 Apr 2025 03:21:32 +0100
Adrián Larumbe <adrian.larumbe@collabora.com> wrote:
> This change is essentially a Panfrost port of commit a3707f53eb3f
> ("drm/panthor: show device-wide list of DRM GEM objects over DebugFS").
>
> The DebugFS file is almost the same as in Panthor, minus the GEM object
> usage flags, since Panfrost has no kernel-only BO's.
>
> Two additional GEM state flags which are displayed but aren't relevant
> to Panthor are 'Purged' and 'Purgeable', since Panfrost implements an
> explicit shrinker and a madvise ioctl to flag objects as reclaimable.
>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
> drivers/gpu/drm/panfrost/panfrost_device.c | 4 +
> drivers/gpu/drm/panfrost/panfrost_device.h | 11 ++
> drivers/gpu/drm/panfrost/panfrost_drv.c | 37 ++++++
> drivers/gpu/drm/panfrost/panfrost_gem.c | 137 +++++++++++++++++++++
> drivers/gpu/drm/panfrost/panfrost_gem.h | 58 +++++++++
> 5 files changed, 247 insertions(+)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index a45e4addcc19..7ba140aaf59d 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -209,6 +209,10 @@ int panfrost_device_init(struct panfrost_device *pfdev)
>
> spin_lock_init(&pfdev->cycle_counter.lock);
>
> +#ifdef CONFIG_DEBUG_FS
> + mutex_init(&pfdev->gems.lock);
> + INIT_LIST_HEAD(&pfdev->gems.node);
> +#endif
> err = panfrost_clk_init(pfdev);
> if (err) {
> dev_err(pfdev->dev, "clk init failed %d\n", err);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> index ad95f2ed31d9..395272a79306 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -161,6 +161,17 @@ struct panfrost_device {
> atomic_t use_count;
> spinlock_t lock;
> } cycle_counter;
> +
> + #ifdef CONFIG_DEBUG_FS
Drop the tab.
> + /** @gems: Device-wide list of GEM objects owned by at least one file. */
> + struct {
> + /** @gems.lock: Protects the device-wide list of GEM objects. */
> + struct mutex lock;
> +
> + /** @node: Used to keep track of all the device's DRM objects */
> + struct list_head node;
> + } gems;
> +#endif
I would probably also put those in a panfrost_device_debugfs struct.
> };
>
> struct panfrost_mmu {
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index b0ab76d67e96..12dd9f311984 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -13,6 +13,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <drm/panfrost_drm.h>
> +#include <drm/drm_debugfs.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_ioctl.h>
> #include <drm/drm_syncobj.h>
> @@ -153,6 +154,8 @@ static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data,
> ret = -EINVAL;
> }
>
> + panfrost_gem_debugfs_init_bo(bo);
This is the only place where you call panfrost_gem_debugfs_init_bo(),
so why not calling panfrost_gem_debugfs_bo_add() at the end of
panfrost_gem_create() instead, and drop the initialised field (and
panfrost_gem_debugfs_init_bo() helper).
> +
> out:
> drm_gem_object_put(&bo->base.base);
> return ret;
> @@ -659,6 +662,37 @@ static const struct file_operations panfrost_drm_driver_fops = {
> .show_fdinfo = drm_show_fdinfo,
> };
>
> +#ifdef CONFIG_DEBUG_FS
> +static int panthor_gems_show(struct seq_file *m, void *data)
> +{
> + struct drm_info_node *node = m->private;
> + struct drm_device *dev = node->minor->dev;
> + struct panfrost_device *pfdev = dev->dev_private;
> +
> + panfrost_gem_debugfs_print_bos(pfdev, m);
> +
> + return 0;
> +}
> +
> +static struct drm_info_list panthor_debugfs_list[] = {
> + {"gems", panthor_gems_show, 0, NULL},
> +};
> +
> +static int panthor_gems_debugfs_init(struct drm_minor *minor)
> +{
> + drm_debugfs_create_files(panthor_debugfs_list,
> + ARRAY_SIZE(panthor_debugfs_list),
> + minor->debugfs_root, minor);
> +
> + return 0;
> +}
> +
> +static void panfrost_debugfs_init(struct drm_minor *minor)
> +{
> + panthor_gems_debugfs_init(minor);
> +}
> +#endif
> +
> /*
> * Panfrost driver version:
> * - 1.0 - initial interface
> @@ -683,6 +717,9 @@ static const struct drm_driver panfrost_drm_driver = {
>
> .gem_create_object = panfrost_gem_create_object,
> .gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table,
> +#ifdef CONFIG_DEBUG_FS
> + .debugfs_init = panfrost_debugfs_init,
> +#endif
> };
>
> static int panfrost_probe(struct platform_device *pdev)
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> index a7a29974d8b1..8a0fd1abd05c 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> @@ -12,6 +12,38 @@
> #include "panfrost_gem.h"
> #include "panfrost_mmu.h"
>
> +#ifdef CONFIG_DEBUG_FS
> +static void panfrost_gem_debugfs_bo_add(struct panfrost_device *ptdev,
> + struct panfrost_gem_object *bo)
> +{
> + INIT_LIST_HEAD(&bo->debugfs.node);
There's no point calling INIT_LIST_HEAD() if you're calling
list_add_tail() immediately after.
> +
> + bo->debugfs.creator.tgid = current->group_leader->pid;
> + get_task_comm(bo->debugfs.creator.process_name, current->group_leader);
> +
> + mutex_lock(&ptdev->gems.lock);
> + list_add_tail(&bo->debugfs.node, &ptdev->gems.node);
> + mutex_unlock(&ptdev->gems.lock);
> +}
> +
> +static void panfrost_gem_debugfs_bo_rm(struct panfrost_gem_object *bo)
> +{
> + struct panfrost_device *ptdev = bo->base.base.dev->dev_private;
> +
> + if (list_empty(&bo->debugfs.node))
> + return;
> +
> + mutex_lock(&ptdev->gems.lock);
> + list_del_init(&bo->debugfs.node);
> + mutex_unlock(&ptdev->gems.lock);
> +}
> +#else
> +static void panfrost_gem_debugfs_bo_add(struct panfrost_device *ptdev,
> + struct panfrost_gem_object *bo)
> +{}
> +static void panfrost_gem_debugfs_bo_rm(struct panfrost_gem_object *bo) {}
> +#endif
> +
> /* Called DRM core on the last userspace/kernel unreference of the
> * BO.
> */
> @@ -36,6 +68,7 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj)
> */
> WARN_ON_ONCE(!list_empty(&bo->mappings.list));
>
> + panfrost_gem_debugfs_bo_rm(bo);
> kfree(bo->label.str);
> mutex_destroy(&bo->label.lock);
>
> @@ -266,6 +299,8 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t
> obj->base.map_wc = !pfdev->coherent;
> mutex_init(&obj->label.lock);
>
> + panfrost_gem_debugfs_bo_add(pfdev, obj);
> +
> return &obj->base.base;
> }
>
> @@ -321,3 +356,105 @@ panfrost_gem_set_label(struct drm_gem_object *obj, const char *label)
>
> kfree(old_label);
> }
> +
> +#ifdef CONFIG_DEBUG_FS
> +struct gem_size_totals {
> + size_t size;
> + size_t resident;
> + size_t reclaimable;
> +};
> +
> +struct flag_def {
> + u32 flag;
> + const char *name;
> +};
> +
> +static void panfrost_gem_debugfs_print_flag_names(struct seq_file *m)
> +{
> + int len;
> + int i;
> +
> + static const struct flag_def gem_state_flags_names[] = {
> + {PANFROST_DEBUGFS_GEM_STATE_FLAG_IMPORTED, "imported"},
> + {PANFROST_DEBUGFS_GEM_STATE_FLAG_EXPORTED, "exported"},
> + {PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGED, "purged"},
> + {PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGEABLE, "purgeable"},
> + };
> +
> + seq_puts(m, "GEM state flags: ");
> + for (i = 0, len = ARRAY_SIZE(gem_state_flags_names); i < len; i++) {
> + seq_printf(m, "%s (0x%x)%s", gem_state_flags_names[i].name,
> + gem_state_flags_names[i].flag, (i < len - 1) ? ", " : "\n\n");
> + }
> +}
> +
> +static void panfrost_gem_debugfs_bo_print(struct panfrost_gem_object *bo,
> + struct seq_file *m,
> + struct gem_size_totals *totals)
> +{
> + unsigned int refcount = kref_read(&bo->base.base.refcount);
> + char creator_info[32] = {};
> + size_t resident_size;
> + u32 gem_state_flags = 0;
> +
> + /* Skip BOs being destroyed. */
> + if (!refcount)
> + return;
> +
> + resident_size = bo->base.pages ? bo->base.base.size : 0;
> +
> + snprintf(creator_info, sizeof(creator_info),
> + "%s/%d", bo->debugfs.creator.process_name, bo->debugfs.creator.tgid);
> + seq_printf(m, "%-32s%-16d%-16d%-16zd%-16zd0x%-16lx",
> + creator_info,
> + bo->base.base.name,
> + refcount,
> + bo->base.base.size,
> + resident_size,
> + drm_vma_node_start(&bo->base.base.vma_node));
> +
> + if (bo->base.base.import_attach)
> + gem_state_flags |= PANFROST_DEBUGFS_GEM_STATE_FLAG_IMPORTED;
> + if (bo->base.base.dma_buf)
> + gem_state_flags |= PANFROST_DEBUGFS_GEM_STATE_FLAG_EXPORTED;
> +
> + if (bo->base.madv < 0)
> + gem_state_flags |= PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGED;
> + else if (bo->base.madv > 0)
> + gem_state_flags |= PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGEABLE;
> +
> + seq_printf(m, "0x%-10x", gem_state_flags);
> +
> + scoped_guard(mutex, &bo->label.lock) {
> + seq_printf(m, "%s\n", bo->label.str ? : "");
> + }
> +
> + totals->size += bo->base.base.size;
> + totals->resident += resident_size;
> + if (bo->base.madv > 0)
> + totals->reclaimable += resident_size;
> +}
> +
> +void panfrost_gem_debugfs_print_bos(struct panfrost_device *ptdev,
> + struct seq_file *m)
> +{
> + struct gem_size_totals totals = {0};
> + struct panfrost_gem_object *bo;
> +
> + panfrost_gem_debugfs_print_flag_names(m);
> +
> + seq_puts(m, "created-by global-name refcount size resident-size file-offset state label\n");
> + seq_puts(m, "-----------------------------------------------------------------------------------------------------------------------------------\n");
> +
> + scoped_guard(mutex, &ptdev->gems.lock) {
> + list_for_each_entry(bo, &ptdev->gems.node, debugfs.node) {
> + if (bo->debugfs.initialised)
> + panfrost_gem_debugfs_bo_print(bo, m, &totals);
> + }
> + }
> +
> + seq_puts(m, "===================================================================================================================================\n");
> + seq_printf(m, "Total size: %zd, Total resident: %zd, Total reclaimable: %zd\n",
> + totals.size, totals.resident, totals.reclaimable);
> +}
> +#endif
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
> index 842e025b9bdc..bc60e0d74da9 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
> @@ -8,9 +8,50 @@
> #include <drm/drm_mm.h>
>
> struct panfrost_mmu;
> +struct panfrost_device;
>
> #define PANFROST_BO_LABEL_MAXLEN 4096
>
> +enum panfrost_debugfs_gem_state_flags {
> + /** @PANFROST_DEBUGFS_GEM_STATE_FLAG_IMPORTED: GEM BO is PRIME imported. */
> + PANFROST_DEBUGFS_GEM_STATE_FLAG_IMPORTED = BIT(0),
> +
> + /** @PANFROST_DEBUGFS_GEM_STATE_FLAG_EXPORTED: GEM BO is PRIME exported. */
> + PANFROST_DEBUGFS_GEM_STATE_FLAG_EXPORTED = BIT(1),
> +
> + /** @PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGED: GEM BO was reclaimed by the shrinker. */
> + PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGED = BIT(2),
> +
> + /**
> + * @PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGEABLE: GEM BO pages were marked as no longer
> + * needed by UM and can be reclaimed by the shrinker.
> + */
> + PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGEABLE = BIT(3),
> +};
> +
> +/**
> + * struct panfrost_gem_debugfs - GEM object's DebugFS list information
> + */
> +struct panfrost_gem_debugfs {
> + /**
> + * @node: Node used to insert the object in the device-wide list of
> + * GEM objects, to display information about it through a DebugFS file.
> + */
> + struct list_head node;
> +
> + /** @creator: Information about the UM process which created the GEM. */
> + struct {
> + /** @creator.process_name: Group leader name in owning thread's process */
> + char process_name[TASK_COMM_LEN];
> +
> + /** @creator.tgid: PID of the thread's group leader within its process */
> + pid_t tgid;
> + } creator;
> +
> + /** @initialised: GEM object is ready to be displayed in DebugFS file. */
> + bool initialised;
> +};
> +
> struct panfrost_gem_object {
> struct drm_gem_shmem_object base;
> struct sg_table *sgts;
> @@ -59,6 +100,10 @@ struct panfrost_gem_object {
>
> bool noexec :1;
> bool is_heap :1;
> +
> +#ifdef CONFIG_DEBUG_FS
> + struct panfrost_gem_debugfs debugfs;
> +#endif
> };
>
> struct panfrost_gem_mapping {
> @@ -107,4 +152,17 @@ void panfrost_gem_shrinker_cleanup(struct drm_device *dev);
>
> void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label);
>
> +#ifdef CONFIG_DEBUG_FS
> +void panfrost_gem_debugfs_print_bos(struct panfrost_device *pfdev,
> + struct seq_file *m);
> +static inline void
> +panfrost_gem_debugfs_init_bo(struct panfrost_gem_object *bo)
> +{
> + bo->debugfs.initialised = true;
> +}
> +#else
> +static inline void
> +panfrost_gem_debugfs_init_bo(struct panfrost_gem_object *bo) {};
> +#endif
> +
> #endif /* __PANFROST_GEM_H__ */
next prev parent reply other threads:[~2025-05-06 7:04 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-24 2:21 [PATCH 0/3] Panfrost BO tagging and GEMS debug display Adrián Larumbe
2025-04-24 2:21 ` [PATCH 1/3] drm/panfrost: Add BO labelling to Panfrost Adrián Larumbe
2025-04-29 16:16 ` Boris Brezillon
2025-05-06 6:54 ` Boris Brezillon
2025-05-07 13:01 ` Adrián Larumbe
2025-05-07 14:25 ` Boris Brezillon
2025-05-07 13:18 ` Steven Price
2025-05-07 14:12 ` Adrián Larumbe
2025-04-24 2:21 ` [PATCH 2/3] drm/panfrost: Add driver IOCTL for setting BO labels Adrián Larumbe
2025-05-06 6:56 ` Boris Brezillon
2025-05-07 13:18 ` Steven Price
2025-04-24 2:21 ` [PATCH 3/3] drm/panfrost: show device-wide list of DRM GEM objects over DebugFS Adrián Larumbe
2025-05-06 7:04 ` Boris Brezillon [this message]
2025-05-07 14:09 ` Adrián Larumbe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250506090418.3c8242b0@collabora.com \
--to=boris.brezillon@collabora.com \
--cc=adrian.larumbe@collabora.com \
--cc=airlied@gmail.com \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=kernel@collabora.com \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=robh@kernel.org \
--cc=simona@ffwll.ch \
--cc=steven.price@arm.com \
--cc=sumit.semwal@linaro.org \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.