* [PATCH 0/3] Panfrost BO tagging and GEMS debug display
@ 2025-04-24 2:21 Adrián Larumbe
2025-04-24 2:21 ` [PATCH 1/3] drm/panfrost: Add BO labelling to Panfrost Adrián Larumbe
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Adrián Larumbe @ 2025-04-24 2:21 UTC (permalink / raw)
To: linux-kernel; +Cc: dri-devel, Boris Brezillon, kernel, Adrián Larumbe
This patch series is a Panfrost port of the already merged patches
previously discussed at [1].
The differences are minimal. However, Panfrost doesn't have Kernel-only BO's, so all the
functionality that dealt with them has been removed.
The under-discussion Mesa MR that would allow one to test these changes can be found at [2].
The way BO flags is printed is also slightly different.
[1] https://lore.kernel.org/dri-devel/20250418022710.74749-1-adrian.larumbe@collabora.com/
[2] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34224
Adrián Larumbe (3):
drm/panfrost: Add BO labelling to Panfrost
drm/panfrost: Add driver IOCTL for setting BO labels
drm/panfrost: show device-wide list of DRM GEM objects over DebugFS
drivers/gpu/drm/panfrost/panfrost_device.c | 4 +
drivers/gpu/drm/panfrost/panfrost_device.h | 11 ++
drivers/gpu/drm/panfrost/panfrost_drv.c | 81 ++++++++++-
drivers/gpu/drm/panfrost/panfrost_gem.c | 156 +++++++++++++++++++++
drivers/gpu/drm/panfrost/panfrost_gem.h | 76 ++++++++++
include/uapi/drm/panfrost_drm.h | 20 +++
6 files changed, 347 insertions(+), 1 deletion(-)
base-commit: a3707f53eb3f4f3e7a30d720be0885f813d649bb
--
2.48.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] drm/panfrost: Add BO labelling to Panfrost 2025-04-24 2:21 [PATCH 0/3] Panfrost BO tagging and GEMS debug display Adrián Larumbe @ 2025-04-24 2:21 ` Adrián Larumbe 2025-04-29 16:16 ` Boris Brezillon ` (2 more replies) 2025-04-24 2:21 ` [PATCH 2/3] drm/panfrost: Add driver IOCTL for setting BO labels Adrián Larumbe 2025-04-24 2:21 ` [PATCH 3/3] drm/panfrost: show device-wide list of DRM GEM objects over DebugFS Adrián Larumbe 2 siblings, 3 replies; 14+ messages in thread From: Adrián Larumbe @ 2025-04-24 2:21 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter Unlike in Panthor, from where this change is based on, there is no need to support tagging of BO's other than UM-exposed ones, so all strings can be freed with kfree(). This commit is done in preparation of a following one that will allow UM to set BO labels through a new ioctl(). Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_gem.c | 19 +++++++++++++++++++ drivers/gpu/drm/panfrost/panfrost_gem.h | 16 ++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c index 963f04ba2de6..a7a29974d8b1 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.c +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */ +#include <linux/cleanup.h> #include <linux/err.h> #include <linux/slab.h> #include <linux/dma-buf.h> @@ -35,6 +36,9 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) */ WARN_ON_ONCE(!list_empty(&bo->mappings.list)); + kfree(bo->label.str); + mutex_destroy(&bo->label.lock); + if (bo->sgts) { int i; int n_sgt = bo->base.base.size / SZ_2M; @@ -260,6 +264,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t mutex_init(&obj->mappings.lock); obj->base.base.funcs = &panfrost_gem_funcs; obj->base.map_wc = !pfdev->coherent; + mutex_init(&obj->label.lock); return &obj->base.base; } @@ -302,3 +307,17 @@ panfrost_gem_prime_import_sg_table(struct drm_device *dev, return obj; } + +void +panfrost_gem_set_label(struct drm_gem_object *obj, const char *label) +{ + struct panfrost_gem_object *bo = to_panfrost_bo(obj); + const char *old_label; + + scoped_guard(mutex, &bo->label.lock) { + old_label = bo->label.str; + bo->label.str = label; + } + + kfree(old_label); +} diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h index 7516b7ecf7fe..c0be2934f229 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.h +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h @@ -41,6 +41,20 @@ struct panfrost_gem_object { */ size_t heap_rss_size; + /** + * @label: BO tagging fields. The label can be assigned within the + * driver itself or through a specific IOCTL. + */ + struct { + /** + * @label.str: Pointer to NULL-terminated string, + */ + const char *str; + + /** @lock.str: Protects access to the @label.str field. */ + struct mutex lock; + } label; + bool noexec :1; bool is_heap :1; }; @@ -89,4 +103,6 @@ void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); int panfrost_gem_shrinker_init(struct drm_device *dev); void panfrost_gem_shrinker_cleanup(struct drm_device *dev); +void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label); + #endif /* __PANFROST_GEM_H__ */ -- 2.48.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/panfrost: Add BO labelling to Panfrost 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:18 ` Steven Price 2 siblings, 0 replies; 14+ messages in thread From: Boris Brezillon @ 2025-04-29 16:16 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, kernel, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Thu, 24 Apr 2025 03:21:30 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Unlike in Panthor, from where this change is based on, there is no need > to support tagging of BO's other than UM-exposed ones, so all strings > can be freed with kfree(). > > This commit is done in preparation of a following one that will allow > UM to set BO labels through a new ioctl(). > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_gem.c | 19 +++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_gem.h | 16 ++++++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 963f04ba2de6..a7a29974d8b1 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */ > > +#include <linux/cleanup.h> > #include <linux/err.h> > #include <linux/slab.h> > #include <linux/dma-buf.h> > @@ -35,6 +36,9 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) > */ > WARN_ON_ONCE(!list_empty(&bo->mappings.list)); > > + kfree(bo->label.str); > + mutex_destroy(&bo->label.lock); > + > if (bo->sgts) { > int i; > int n_sgt = bo->base.base.size / SZ_2M; > @@ -260,6 +264,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t > mutex_init(&obj->mappings.lock); > obj->base.base.funcs = &panfrost_gem_funcs; > obj->base.map_wc = !pfdev->coherent; > + mutex_init(&obj->label.lock); > > return &obj->base.base; > } > @@ -302,3 +307,17 @@ panfrost_gem_prime_import_sg_table(struct drm_device *dev, > > return obj; > } > + > +void > +panfrost_gem_set_label(struct drm_gem_object *obj, const char *label) > +{ > + struct panfrost_gem_object *bo = to_panfrost_bo(obj); > + const char *old_label; > + > + scoped_guard(mutex, &bo->label.lock) { > + old_label = bo->label.str; > + bo->label.str = label; > + } > + > + kfree(old_label); > +} > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index 7516b7ecf7fe..c0be2934f229 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -41,6 +41,20 @@ struct panfrost_gem_object { > */ > size_t heap_rss_size; > > + /** > + * @label: BO tagging fields. The label can be assigned within the > + * driver itself or through a specific IOCTL. > + */ > + struct { > + /** > + * @label.str: Pointer to NULL-terminated string, > + */ > + const char *str; > + > + /** @lock.str: Protects access to the @label.str field. */ > + struct mutex lock; > + } label; Should we have a panfrost_gem_debugfs where we put all the debugfs fields, like we have in panthor? > + > bool noexec :1; > bool is_heap :1; > }; > @@ -89,4 +103,6 @@ void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > +void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label); > + > #endif /* __PANFROST_GEM_H__ */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/panfrost: Add BO labelling to Panfrost 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 13:18 ` Steven Price 2 siblings, 1 reply; 14+ messages in thread From: Boris Brezillon @ 2025-05-06 6:54 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, kernel, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Thu, 24 Apr 2025 03:21:30 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Unlike in Panthor, from where this change is based on, there is no need > to support tagging of BO's other than UM-exposed ones, so all strings > can be freed with kfree(). > > This commit is done in preparation of a following one that will allow > UM to set BO labels through a new ioctl(). > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_gem.c | 19 +++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_gem.h | 16 ++++++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 963f04ba2de6..a7a29974d8b1 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */ > > +#include <linux/cleanup.h> > #include <linux/err.h> > #include <linux/slab.h> > #include <linux/dma-buf.h> > @@ -35,6 +36,9 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) > */ > WARN_ON_ONCE(!list_empty(&bo->mappings.list)); > > + kfree(bo->label.str); > + mutex_destroy(&bo->label.lock); > + > if (bo->sgts) { > int i; > int n_sgt = bo->base.base.size / SZ_2M; > @@ -260,6 +264,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t > mutex_init(&obj->mappings.lock); > obj->base.base.funcs = &panfrost_gem_funcs; > obj->base.map_wc = !pfdev->coherent; > + mutex_init(&obj->label.lock); > > return &obj->base.base; > } > @@ -302,3 +307,17 @@ panfrost_gem_prime_import_sg_table(struct drm_device *dev, > > return obj; > } > + > +void > +panfrost_gem_set_label(struct drm_gem_object *obj, const char *label) > +{ > + struct panfrost_gem_object *bo = to_panfrost_bo(obj); > + const char *old_label; > + > + scoped_guard(mutex, &bo->label.lock) { > + old_label = bo->label.str; > + bo->label.str = label; > + } > + > + kfree(old_label); > +} > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index 7516b7ecf7fe..c0be2934f229 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -41,6 +41,20 @@ struct panfrost_gem_object { > */ > size_t heap_rss_size; > > + /** > + * @label: BO tagging fields. The label can be assigned within the > + * driver itself or through a specific IOCTL. > + */ > + struct { > + /** > + * @label.str: Pointer to NULL-terminated string, > + */ > + const char *str; > + > + /** @lock.str: Protects access to the @label.str field. */ > + struct mutex lock; > + } label; Can we do as we did in panthor, and put those fields in the debugfs field. > + > bool noexec :1; > bool is_heap :1; > }; > @@ -89,4 +103,6 @@ void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > +void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label); > + > #endif /* __PANFROST_GEM_H__ */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/panfrost: Add BO labelling to Panfrost 2025-05-06 6:54 ` Boris Brezillon @ 2025-05-07 13:01 ` Adrián Larumbe 2025-05-07 14:25 ` Boris Brezillon 0 siblings, 1 reply; 14+ messages in thread From: Adrián Larumbe @ 2025-05-07 13:01 UTC (permalink / raw) To: Boris Brezillon Cc: linux-kernel, dri-devel, kernel, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On 06.05.2025 08:54, Boris Brezillon wrote: > On Thu, 24 Apr 2025 03:21:30 +0100 > Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > > > Unlike in Panthor, from where this change is based on, there is no need > > to support tagging of BO's other than UM-exposed ones, so all strings > > can be freed with kfree(). > > > > This commit is done in preparation of a following one that will allow > > UM to set BO labels through a new ioctl(). > > > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > > --- > > drivers/gpu/drm/panfrost/panfrost_gem.c | 19 +++++++++++++++++++ > > drivers/gpu/drm/panfrost/panfrost_gem.h | 16 ++++++++++++++++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > > index 963f04ba2de6..a7a29974d8b1 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > > @@ -1,6 +1,7 @@ > > // SPDX-License-Identifier: GPL-2.0 > > /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */ > > > > +#include <linux/cleanup.h> > > #include <linux/err.h> > > #include <linux/slab.h> > > #include <linux/dma-buf.h> > > @@ -35,6 +36,9 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) > > */ > > WARN_ON_ONCE(!list_empty(&bo->mappings.list)); > > > > + kfree(bo->label.str); > > + mutex_destroy(&bo->label.lock); > > + > > if (bo->sgts) { > > int i; > > int n_sgt = bo->base.base.size / SZ_2M; > > @@ -260,6 +264,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t > > mutex_init(&obj->mappings.lock); > > obj->base.base.funcs = &panfrost_gem_funcs; > > obj->base.map_wc = !pfdev->coherent; > > + mutex_init(&obj->label.lock); > > > > return &obj->base.base; > > } > > @@ -302,3 +307,17 @@ panfrost_gem_prime_import_sg_table(struct drm_device *dev, > > > > return obj; > > } > > + > > +void > > +panfrost_gem_set_label(struct drm_gem_object *obj, const char *label) > > +{ > > + struct panfrost_gem_object *bo = to_panfrost_bo(obj); > > + const char *old_label; > > + > > + scoped_guard(mutex, &bo->label.lock) { > > + old_label = bo->label.str; > > + bo->label.str = label; > > + } > > + > > + kfree(old_label); > > +} > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > > index 7516b7ecf7fe..c0be2934f229 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > > @@ -41,6 +41,20 @@ struct panfrost_gem_object { > > */ > > size_t heap_rss_size; > > > > + /** > > + * @label: BO tagging fields. The label can be assigned within the > > + * driver itself or through a specific IOCTL. > > + */ > > + struct { > > + /** > > + * @label.str: Pointer to NULL-terminated string, > > + */ > > + const char *str; > > + > > + /** @lock.str: Protects access to the @label.str field. */ > > + struct mutex lock; > > + } label; > > Can we do as we did in panthor, and put those fields in the debugfs > field. BO labelling fields must be present outside of DebugFS builds. > + > bool noexec :1; > bool is_heap :1; > }; > @@ -89,4 +103,6 @@ void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > +void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label); > + > #endif /* __PANFROST_GEM_H__ */ Adrian Larumbe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/panfrost: Add BO labelling to Panfrost 2025-05-07 13:01 ` Adrián Larumbe @ 2025-05-07 14:25 ` Boris Brezillon 0 siblings, 0 replies; 14+ messages in thread From: Boris Brezillon @ 2025-05-07 14:25 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, kernel, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Wed, 7 May 2025 14:01:04 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > On 06.05.2025 08:54, Boris Brezillon wrote: > > On Thu, 24 Apr 2025 03:21:30 +0100 > > Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > > > > > Unlike in Panthor, from where this change is based on, there is no need > > > to support tagging of BO's other than UM-exposed ones, so all strings > > > can be freed with kfree(). > > > > > > This commit is done in preparation of a following one that will allow > > > UM to set BO labels through a new ioctl(). > > > > > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > > > --- > > > drivers/gpu/drm/panfrost/panfrost_gem.c | 19 +++++++++++++++++++ > > > drivers/gpu/drm/panfrost/panfrost_gem.h | 16 ++++++++++++++++ > > > 2 files changed, 35 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > > > index 963f04ba2de6..a7a29974d8b1 100644 > > > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > > > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > > > @@ -1,6 +1,7 @@ > > > // SPDX-License-Identifier: GPL-2.0 > > > /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */ > > > > > > +#include <linux/cleanup.h> > > > #include <linux/err.h> > > > #include <linux/slab.h> > > > #include <linux/dma-buf.h> > > > @@ -35,6 +36,9 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) > > > */ > > > WARN_ON_ONCE(!list_empty(&bo->mappings.list)); > > > > > > + kfree(bo->label.str); > > > + mutex_destroy(&bo->label.lock); > > > + > > > if (bo->sgts) { > > > int i; > > > int n_sgt = bo->base.base.size / SZ_2M; > > > @@ -260,6 +264,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t > > > mutex_init(&obj->mappings.lock); > > > obj->base.base.funcs = &panfrost_gem_funcs; > > > obj->base.map_wc = !pfdev->coherent; > > > + mutex_init(&obj->label.lock); > > > > > > return &obj->base.base; > > > } > > > @@ -302,3 +307,17 @@ panfrost_gem_prime_import_sg_table(struct drm_device *dev, > > > > > > return obj; > > > } > > > + > > > +void > > > +panfrost_gem_set_label(struct drm_gem_object *obj, const char *label) > > > +{ > > > + struct panfrost_gem_object *bo = to_panfrost_bo(obj); > > > + const char *old_label; > > > + > > > + scoped_guard(mutex, &bo->label.lock) { > > > + old_label = bo->label.str; > > > + bo->label.str = label; > > > + } > > > + > > > + kfree(old_label); > > > +} > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > > > index 7516b7ecf7fe..c0be2934f229 100644 > > > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > > > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > > > @@ -41,6 +41,20 @@ struct panfrost_gem_object { > > > */ > > > size_t heap_rss_size; > > > > > > + /** > > > + * @label: BO tagging fields. The label can be assigned within the > > > + * driver itself or through a specific IOCTL. > > > + */ > > > + struct { > > > + /** > > > + * @label.str: Pointer to NULL-terminated string, > > > + */ > > > + const char *str; > > > + > > > + /** @lock.str: Protects access to the @label.str field. */ > > > + struct mutex lock; > > > + } label; > > > > Can we do as we did in panthor, and put those fields in the debugfs > > field. > > BO labelling fields must be present outside of DebugFS builds. Hm, right. I see those are not in an #ifdef CONFIG_DEBUGFS section in panthor. I don't really see a good reason to store labels if we don't have a way to retrieve/display them, but I guess that'd make sense if we introduce a GET_LABEL ioctl. > > > + > > bool noexec :1; > > bool is_heap :1; > > }; > > @@ -89,4 +103,6 @@ void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > > int panfrost_gem_shrinker_init(struct drm_device *dev); > > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > > > +void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label); > > + > > #endif /* __PANFROST_GEM_H__ */ > > > Adrian Larumbe ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/panfrost: Add BO labelling to Panfrost 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:18 ` Steven Price 2025-05-07 14:12 ` Adrián Larumbe 2 siblings, 1 reply; 14+ messages in thread From: Steven Price @ 2025-05-07 13:18 UTC (permalink / raw) To: Adrián Larumbe, linux-kernel Cc: dri-devel, Boris Brezillon, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On 24/04/2025 03:21, Adrián Larumbe wrote: > Unlike in Panthor, from where this change is based on, there is no need > to support tagging of BO's other than UM-exposed ones, so all strings > can be freed with kfree(). > > This commit is done in preparation of a following one that will allow > UM to set BO labels through a new ioctl(). > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> Reviewed-by: Steven Price <steven.price@arm.com> But one comment below > --- > drivers/gpu/drm/panfrost/panfrost_gem.c | 19 +++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_gem.h | 16 ++++++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 963f04ba2de6..a7a29974d8b1 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */ > > +#include <linux/cleanup.h> > #include <linux/err.h> > #include <linux/slab.h> > #include <linux/dma-buf.h> > @@ -35,6 +36,9 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) > */ > WARN_ON_ONCE(!list_empty(&bo->mappings.list)); > > + kfree(bo->label.str); > + mutex_destroy(&bo->label.lock); > + > if (bo->sgts) { > int i; > int n_sgt = bo->base.base.size / SZ_2M; > @@ -260,6 +264,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t > mutex_init(&obj->mappings.lock); > obj->base.base.funcs = &panfrost_gem_funcs; > obj->base.map_wc = !pfdev->coherent; > + mutex_init(&obj->label.lock); > > return &obj->base.base; > } > @@ -302,3 +307,17 @@ panfrost_gem_prime_import_sg_table(struct drm_device *dev, > > return obj; > } > + > +void > +panfrost_gem_set_label(struct drm_gem_object *obj, const char *label) > +{ > + struct panfrost_gem_object *bo = to_panfrost_bo(obj); > + const char *old_label; > + > + scoped_guard(mutex, &bo->label.lock) { > + old_label = bo->label.str; > + bo->label.str = label; > + } > + > + kfree(old_label); > +} > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index 7516b7ecf7fe..c0be2934f229 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -41,6 +41,20 @@ struct panfrost_gem_object { > */ > size_t heap_rss_size; > > + /** > + * @label: BO tagging fields. The label can be assigned within the > + * driver itself or through a specific IOCTL. From the commit message (about the use of kfree()) I assume we are not expecting this to be assigned "within the driver itself"? Thanks, Steve > + */ > + struct { > + /** > + * @label.str: Pointer to NULL-terminated string, > + */ > + const char *str; > + > + /** @lock.str: Protects access to the @label.str field. */ > + struct mutex lock; > + } label; > + > bool noexec :1; > bool is_heap :1; > }; > @@ -89,4 +103,6 @@ void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > int panfrost_gem_shrinker_init(struct drm_device *dev); > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > +void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label); > + > #endif /* __PANFROST_GEM_H__ */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/panfrost: Add BO labelling to Panfrost 2025-05-07 13:18 ` Steven Price @ 2025-05-07 14:12 ` Adrián Larumbe 0 siblings, 0 replies; 14+ messages in thread From: Adrián Larumbe @ 2025-05-07 14:12 UTC (permalink / raw) To: Steven Price Cc: linux-kernel, dri-devel, Boris Brezillon, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On 07.05.2025 14:18, Steven Price wrote: > On 24/04/2025 03:21, Adrián Larumbe wrote: > > Unlike in Panthor, from where this change is based on, there is no need > > to support tagging of BO's other than UM-exposed ones, so all strings > > can be freed with kfree(). > > > > This commit is done in preparation of a following one that will allow > > UM to set BO labels through a new ioctl(). > > > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> > > Reviewed-by: Steven Price <steven.price@arm.com> > > But one comment below > > > --- > > drivers/gpu/drm/panfrost/panfrost_gem.c | 19 +++++++++++++++++++ > > drivers/gpu/drm/panfrost/panfrost_gem.h | 16 ++++++++++++++++ > > 2 files changed, 35 insertions(+) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > > index 963f04ba2de6..a7a29974d8b1 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > > @@ -1,6 +1,7 @@ > > // SPDX-License-Identifier: GPL-2.0 > > /* Copyright 2019 Linaro, Ltd, Rob Herring <robh@kernel.org> */ > > > > +#include <linux/cleanup.h> > > #include <linux/err.h> > > #include <linux/slab.h> > > #include <linux/dma-buf.h> > > @@ -35,6 +36,9 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) > > */ > > WARN_ON_ONCE(!list_empty(&bo->mappings.list)); > > > > + kfree(bo->label.str); > > + mutex_destroy(&bo->label.lock); > > + > > if (bo->sgts) { > > int i; > > int n_sgt = bo->base.base.size / SZ_2M; > > @@ -260,6 +264,7 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t > > mutex_init(&obj->mappings.lock); > > obj->base.base.funcs = &panfrost_gem_funcs; > > obj->base.map_wc = !pfdev->coherent; > > + mutex_init(&obj->label.lock); > > > > return &obj->base.base; > > } > > @@ -302,3 +307,17 @@ panfrost_gem_prime_import_sg_table(struct drm_device *dev, > > > > return obj; > > } > > + > > +void > > +panfrost_gem_set_label(struct drm_gem_object *obj, const char *label) > > +{ > > + struct panfrost_gem_object *bo = to_panfrost_bo(obj); > > + const char *old_label; > > + > > + scoped_guard(mutex, &bo->label.lock) { > > + old_label = bo->label.str; > > + bo->label.str = label; > > + } > > + > > + kfree(old_label); > > +} > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > > index 7516b7ecf7fe..c0be2934f229 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > > @@ -41,6 +41,20 @@ struct panfrost_gem_object { > > */ > > size_t heap_rss_size; > > > > + /** > > + * @label: BO tagging fields. The label can be assigned within the > > + * driver itself or through a specific IOCTL. > > From the commit message (about the use of kfree()) I assume we are not > expecting this to be assigned "within the driver itself"? > That's right, that was only the case in Panthor when labelling kernel BOs. Since all GEM BO's in Panfrost are user-facing, there's no need to label any within the driver itself. > > Thanks, > Steve > > > + */ > > + struct { > > + /** > > + * @label.str: Pointer to NULL-terminated string, > > + */ > > + const char *str; > > + > > + /** @lock.str: Protects access to the @label.str field. */ > > + struct mutex lock; > > + } label; > > + > > bool noexec :1; > > bool is_heap :1; > > }; > > @@ -89,4 +103,6 @@ void panfrost_gem_teardown_mappings_locked(struct panfrost_gem_object *bo); > > int panfrost_gem_shrinker_init(struct drm_device *dev); > > void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > > > > +void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label); > > + > > #endif /* __PANFROST_GEM_H__ */ Adrian Larumbe ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] drm/panfrost: Add driver IOCTL for setting BO labels 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-24 2:21 ` 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 2 siblings, 2 replies; 14+ messages in thread From: Adrián Larumbe @ 2025-04-24 2:21 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter Allow UM to label a BO for which it possesses a DRM handle. Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_drv.c | 44 ++++++++++++++++++++++++- drivers/gpu/drm/panfrost/panfrost_gem.h | 2 ++ include/uapi/drm/panfrost_drm.h | 20 +++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index b87f83e94eda..b0ab76d67e96 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -495,6 +495,46 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, return ret; } +static int panfrost_ioctl_set_label_bo(struct drm_device *ddev, void *data, + struct drm_file *file) +{ + struct drm_panfrost_set_label_bo *args = data; + struct drm_gem_object *obj; + const char *label = NULL; + int ret = 0; + + if (args->pad) + return -EINVAL; + + obj = drm_gem_object_lookup(file, args->handle); + if (!obj) + return -ENOENT; + + if (args->label) { + label = strndup_user((const char __user *)(uintptr_t)args->label, + PANFROST_BO_LABEL_MAXLEN); + if (IS_ERR(label)) { + ret = PTR_ERR(label); + if (ret == -EINVAL) + ret = -E2BIG; + goto err_put_obj; + } + } + + /* + * We treat passing a label of length 0 and passing a NULL label + * differently, because even though they might seem conceptually + * similar, future uses of the BO label might expect a different + * behaviour in each case. + */ + panfrost_gem_set_label(obj, label); + +err_put_obj: + drm_gem_object_put(obj); + + return ret; +} + int panfrost_unstable_ioctl_check(void) { if (!unstable_ioctls) @@ -561,6 +601,7 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { PANFROST_IOCTL(PERFCNT_ENABLE, perfcnt_enable, DRM_RENDER_ALLOW), PANFROST_IOCTL(PERFCNT_DUMP, perfcnt_dump, DRM_RENDER_ALLOW), PANFROST_IOCTL(MADVISE, madvise, DRM_RENDER_ALLOW), + PANFROST_IOCTL(SET_LABEL_BO, set_label_bo, DRM_RENDER_ALLOW), }; static void panfrost_gpu_show_fdinfo(struct panfrost_device *pfdev, @@ -625,6 +666,7 @@ static const struct file_operations panfrost_drm_driver_fops = { * - 1.2 - adds AFBC_FEATURES query * - 1.3 - adds JD_REQ_CYCLE_COUNT job requirement for SUBMIT * - adds SYSTEM_TIMESTAMP and SYSTEM_TIMESTAMP_FREQUENCY queries + * - 1.4 - adds SET_LABEL_BO */ static const struct drm_driver panfrost_drm_driver = { .driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ, @@ -637,7 +679,7 @@ static const struct drm_driver panfrost_drm_driver = { .name = "panfrost", .desc = "panfrost DRM", .major = 1, - .minor = 3, + .minor = 4, .gem_create_object = panfrost_gem_create_object, .gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table, diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h index c0be2934f229..842e025b9bdc 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gem.h +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h @@ -9,6 +9,8 @@ struct panfrost_mmu; +#define PANFROST_BO_LABEL_MAXLEN 4096 + struct panfrost_gem_object { struct drm_gem_shmem_object base; struct sg_table *sgts; diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h index 568724be6628..b0445c5e514d 100644 --- a/include/uapi/drm/panfrost_drm.h +++ b/include/uapi/drm/panfrost_drm.h @@ -21,6 +21,7 @@ extern "C" { #define DRM_PANFROST_PERFCNT_ENABLE 0x06 #define DRM_PANFROST_PERFCNT_DUMP 0x07 #define DRM_PANFROST_MADVISE 0x08 +#define DRM_PANFROST_SET_LABEL_BO 0x09 #define DRM_IOCTL_PANFROST_SUBMIT DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_SUBMIT, struct drm_panfrost_submit) #define DRM_IOCTL_PANFROST_WAIT_BO DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_WAIT_BO, struct drm_panfrost_wait_bo) @@ -29,6 +30,7 @@ extern "C" { #define DRM_IOCTL_PANFROST_GET_PARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_GET_PARAM, struct drm_panfrost_get_param) #define DRM_IOCTL_PANFROST_GET_BO_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_GET_BO_OFFSET, struct drm_panfrost_get_bo_offset) #define DRM_IOCTL_PANFROST_MADVISE DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_MADVISE, struct drm_panfrost_madvise) +#define DRM_IOCTL_PANFROST_SET_LABEL_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_SET_LABEL_BO, struct drm_panfrost_set_label_bo) /* * Unstable ioctl(s): only exposed when the unsafe unstable_ioctls module @@ -227,6 +229,24 @@ struct drm_panfrost_madvise { __u32 retained; /* out, whether backing store still exists */ }; +/** + * struct drm_panfrost_set_label_bo - ioctl argument for labelling Panfrost BOs. + */ +struct drm_panfrost_set_label_bo { + /** @handle: Handle of the buffer object to label. */ + __u32 handle; + + /** @pad: MBZ. */ + __u32 pad; + + /** + * @label: User pointer to a NUL-terminated string + * + * Length cannot be greater than 4096 + */ + __u64 label; +}; + /* Definitions for coredump decoding in user space */ #define PANFROSTDUMP_MAJOR 1 #define PANFROSTDUMP_MINOR 0 -- 2.48.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] drm/panfrost: Add driver IOCTL for setting BO labels 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 1 sibling, 0 replies; 14+ messages in thread From: Boris Brezillon @ 2025-05-06 6:56 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, kernel, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On Thu, 24 Apr 2025 03:21:31 +0100 Adrián Larumbe <adrian.larumbe@collabora.com> wrote: > Allow UM to label a BO for which it possesses a DRM handle. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 44 ++++++++++++++++++++++++- > drivers/gpu/drm/panfrost/panfrost_gem.h | 2 ++ > include/uapi/drm/panfrost_drm.h | 20 +++++++++++ > 3 files changed, 65 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index b87f83e94eda..b0ab76d67e96 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -495,6 +495,46 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, > return ret; > } > > +static int panfrost_ioctl_set_label_bo(struct drm_device *ddev, void *data, > + struct drm_file *file) > +{ > + struct drm_panfrost_set_label_bo *args = data; > + struct drm_gem_object *obj; > + const char *label = NULL; > + int ret = 0; > + > + if (args->pad) > + return -EINVAL; > + > + obj = drm_gem_object_lookup(file, args->handle); > + if (!obj) > + return -ENOENT; > + > + if (args->label) { > + label = strndup_user((const char __user *)(uintptr_t)args->label, > + PANFROST_BO_LABEL_MAXLEN); > + if (IS_ERR(label)) { > + ret = PTR_ERR(label); > + if (ret == -EINVAL) > + ret = -E2BIG; > + goto err_put_obj; > + } > + } > + > + /* > + * We treat passing a label of length 0 and passing a NULL label > + * differently, because even though they might seem conceptually > + * similar, future uses of the BO label might expect a different > + * behaviour in each case. > + */ > + panfrost_gem_set_label(obj, label); > + > +err_put_obj: > + drm_gem_object_put(obj); > + > + return ret; > +} > + > int panfrost_unstable_ioctl_check(void) > { > if (!unstable_ioctls) > @@ -561,6 +601,7 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { > PANFROST_IOCTL(PERFCNT_ENABLE, perfcnt_enable, DRM_RENDER_ALLOW), > PANFROST_IOCTL(PERFCNT_DUMP, perfcnt_dump, DRM_RENDER_ALLOW), > PANFROST_IOCTL(MADVISE, madvise, DRM_RENDER_ALLOW), > + PANFROST_IOCTL(SET_LABEL_BO, set_label_bo, DRM_RENDER_ALLOW), > }; > > static void panfrost_gpu_show_fdinfo(struct panfrost_device *pfdev, > @@ -625,6 +666,7 @@ static const struct file_operations panfrost_drm_driver_fops = { > * - 1.2 - adds AFBC_FEATURES query > * - 1.3 - adds JD_REQ_CYCLE_COUNT job requirement for SUBMIT > * - adds SYSTEM_TIMESTAMP and SYSTEM_TIMESTAMP_FREQUENCY queries > + * - 1.4 - adds SET_LABEL_BO > */ > static const struct drm_driver panfrost_drm_driver = { > .driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ, > @@ -637,7 +679,7 @@ static const struct drm_driver panfrost_drm_driver = { > .name = "panfrost", > .desc = "panfrost DRM", > .major = 1, > - .minor = 3, > + .minor = 4, > > .gem_create_object = panfrost_gem_create_object, > .gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table, > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index c0be2934f229..842e025b9bdc 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -9,6 +9,8 @@ > > struct panfrost_mmu; > > +#define PANFROST_BO_LABEL_MAXLEN 4096 > + > struct panfrost_gem_object { > struct drm_gem_shmem_object base; > struct sg_table *sgts; > diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h > index 568724be6628..b0445c5e514d 100644 > --- a/include/uapi/drm/panfrost_drm.h > +++ b/include/uapi/drm/panfrost_drm.h > @@ -21,6 +21,7 @@ extern "C" { > #define DRM_PANFROST_PERFCNT_ENABLE 0x06 > #define DRM_PANFROST_PERFCNT_DUMP 0x07 > #define DRM_PANFROST_MADVISE 0x08 > +#define DRM_PANFROST_SET_LABEL_BO 0x09 > > #define DRM_IOCTL_PANFROST_SUBMIT DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_SUBMIT, struct drm_panfrost_submit) > #define DRM_IOCTL_PANFROST_WAIT_BO DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_WAIT_BO, struct drm_panfrost_wait_bo) > @@ -29,6 +30,7 @@ extern "C" { > #define DRM_IOCTL_PANFROST_GET_PARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_GET_PARAM, struct drm_panfrost_get_param) > #define DRM_IOCTL_PANFROST_GET_BO_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_GET_BO_OFFSET, struct drm_panfrost_get_bo_offset) > #define DRM_IOCTL_PANFROST_MADVISE DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_MADVISE, struct drm_panfrost_madvise) > +#define DRM_IOCTL_PANFROST_SET_LABEL_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_SET_LABEL_BO, struct drm_panfrost_set_label_bo) > > /* > * Unstable ioctl(s): only exposed when the unsafe unstable_ioctls module > @@ -227,6 +229,24 @@ struct drm_panfrost_madvise { > __u32 retained; /* out, whether backing store still exists */ > }; > > +/** > + * struct drm_panfrost_set_label_bo - ioctl argument for labelling Panfrost BOs. > + */ > +struct drm_panfrost_set_label_bo { > + /** @handle: Handle of the buffer object to label. */ > + __u32 handle; > + > + /** @pad: MBZ. */ > + __u32 pad; > + > + /** > + * @label: User pointer to a NUL-terminated string > + * > + * Length cannot be greater than 4096 > + */ > + __u64 label; > +}; > + > /* Definitions for coredump decoding in user space */ > #define PANFROSTDUMP_MAJOR 1 > #define PANFROSTDUMP_MINOR 0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] drm/panfrost: Add driver IOCTL for setting BO labels 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 1 sibling, 0 replies; 14+ messages in thread From: Steven Price @ 2025-05-07 13:18 UTC (permalink / raw) To: Adrián Larumbe, linux-kernel Cc: dri-devel, Boris Brezillon, kernel, Rob Herring, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter On 24/04/2025 03:21, Adrián Larumbe wrote: > Allow UM to label a BO for which it possesses a DRM handle. > > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com> Minor comments below, but otherwise: Reviewed-by: Steven Price <steven.price@arm.com> > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 44 ++++++++++++++++++++++++- > drivers/gpu/drm/panfrost/panfrost_gem.h | 2 ++ > include/uapi/drm/panfrost_drm.h | 20 +++++++++++ > 3 files changed, 65 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index b87f83e94eda..b0ab76d67e96 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -495,6 +495,46 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, > return ret; > } > > +static int panfrost_ioctl_set_label_bo(struct drm_device *ddev, void *data, > + struct drm_file *file) > +{ > + struct drm_panfrost_set_label_bo *args = data; > + struct drm_gem_object *obj; > + const char *label = NULL; > + int ret = 0; > + > + if (args->pad) > + return -EINVAL; > + > + obj = drm_gem_object_lookup(file, args->handle); > + if (!obj) > + return -ENOENT; > + > + if (args->label) { > + label = strndup_user((const char __user *)(uintptr_t)args->label, Use u64_to_user_ptr() > + PANFROST_BO_LABEL_MAXLEN); > + if (IS_ERR(label)) { > + ret = PTR_ERR(label); > + if (ret == -EINVAL) > + ret = -E2BIG; > + goto err_put_obj; > + } > + } > + > + /* > + * We treat passing a label of length 0 and passing a NULL label > + * differently, because even though they might seem conceptually > + * similar, future uses of the BO label might expect a different > + * behaviour in each case. > + */ > + panfrost_gem_set_label(obj, label); > + > +err_put_obj: > + drm_gem_object_put(obj); > + > + return ret; > +} > + > int panfrost_unstable_ioctl_check(void) > { > if (!unstable_ioctls) > @@ -561,6 +601,7 @@ static const struct drm_ioctl_desc panfrost_drm_driver_ioctls[] = { > PANFROST_IOCTL(PERFCNT_ENABLE, perfcnt_enable, DRM_RENDER_ALLOW), > PANFROST_IOCTL(PERFCNT_DUMP, perfcnt_dump, DRM_RENDER_ALLOW), > PANFROST_IOCTL(MADVISE, madvise, DRM_RENDER_ALLOW), > + PANFROST_IOCTL(SET_LABEL_BO, set_label_bo, DRM_RENDER_ALLOW), > }; > > static void panfrost_gpu_show_fdinfo(struct panfrost_device *pfdev, > @@ -625,6 +666,7 @@ static const struct file_operations panfrost_drm_driver_fops = { > * - 1.2 - adds AFBC_FEATURES query > * - 1.3 - adds JD_REQ_CYCLE_COUNT job requirement for SUBMIT > * - adds SYSTEM_TIMESTAMP and SYSTEM_TIMESTAMP_FREQUENCY queries > + * - 1.4 - adds SET_LABEL_BO > */ > static const struct drm_driver panfrost_drm_driver = { > .driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ, > @@ -637,7 +679,7 @@ static const struct drm_driver panfrost_drm_driver = { > .name = "panfrost", > .desc = "panfrost DRM", > .major = 1, > - .minor = 3, > + .minor = 4, > > .gem_create_object = panfrost_gem_create_object, > .gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table, > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index c0be2934f229..842e025b9bdc 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -9,6 +9,8 @@ > > struct panfrost_mmu; > > +#define PANFROST_BO_LABEL_MAXLEN 4096 > + > struct panfrost_gem_object { > struct drm_gem_shmem_object base; > struct sg_table *sgts; > diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h > index 568724be6628..b0445c5e514d 100644 > --- a/include/uapi/drm/panfrost_drm.h > +++ b/include/uapi/drm/panfrost_drm.h > @@ -21,6 +21,7 @@ extern "C" { > #define DRM_PANFROST_PERFCNT_ENABLE 0x06 > #define DRM_PANFROST_PERFCNT_DUMP 0x07 > #define DRM_PANFROST_MADVISE 0x08 > +#define DRM_PANFROST_SET_LABEL_BO 0x09 > > #define DRM_IOCTL_PANFROST_SUBMIT DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_SUBMIT, struct drm_panfrost_submit) > #define DRM_IOCTL_PANFROST_WAIT_BO DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_WAIT_BO, struct drm_panfrost_wait_bo) > @@ -29,6 +30,7 @@ extern "C" { > #define DRM_IOCTL_PANFROST_GET_PARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_GET_PARAM, struct drm_panfrost_get_param) > #define DRM_IOCTL_PANFROST_GET_BO_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_GET_BO_OFFSET, struct drm_panfrost_get_bo_offset) > #define DRM_IOCTL_PANFROST_MADVISE DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_MADVISE, struct drm_panfrost_madvise) > +#define DRM_IOCTL_PANFROST_SET_LABEL_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_SET_LABEL_BO, struct drm_panfrost_set_label_bo) > > /* > * Unstable ioctl(s): only exposed when the unsafe unstable_ioctls module > @@ -227,6 +229,24 @@ struct drm_panfrost_madvise { > __u32 retained; /* out, whether backing store still exists */ > }; > > +/** > + * struct drm_panfrost_set_label_bo - ioctl argument for labelling Panfrost BOs. > + */ > +struct drm_panfrost_set_label_bo { > + /** @handle: Handle of the buffer object to label. */ > + __u32 handle; > + > + /** @pad: MBZ. */ > + __u32 pad; > + > + /** > + * @label: User pointer to a NUL-terminated string > + * > + * Length cannot be greater than 4096 NULL is permitted and means clear the label. Thanks, Steve > + */ > + __u64 label; > +}; > + > /* Definitions for coredump decoding in user space */ > #define PANFROSTDUMP_MAJOR 1 > #define PANFROSTDUMP_MINOR 0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] drm/panfrost: show device-wide list of DRM GEM objects over DebugFS 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-24 2:21 ` [PATCH 2/3] drm/panfrost: Add driver IOCTL for setting BO labels Adrián Larumbe @ 2025-04-24 2:21 ` Adrián Larumbe 2025-05-06 7:04 ` Boris Brezillon 2 siblings, 1 reply; 14+ messages in thread From: Adrián Larumbe @ 2025-04-24 2:21 UTC (permalink / raw) To: linux-kernel Cc: dri-devel, Boris Brezillon, kernel, Adrián Larumbe, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal, Christian König, linux-media, linaro-mm-sig 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 + /** @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 }; 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); + 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); + + 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__ */ -- 2.48.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drm/panfrost: show device-wide list of DRM GEM objects over DebugFS 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 2025-05-07 14:09 ` Adrián Larumbe 0 siblings, 1 reply; 14+ messages in thread From: Boris Brezillon @ 2025-05-06 7:04 UTC (permalink / raw) To: Adrián Larumbe Cc: linux-kernel, dri-devel, kernel, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal, Christian König, linux-media, linaro-mm-sig 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__ */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drm/panfrost: show device-wide list of DRM GEM objects over DebugFS 2025-05-06 7:04 ` Boris Brezillon @ 2025-05-07 14:09 ` Adrián Larumbe 0 siblings, 0 replies; 14+ messages in thread From: Adrián Larumbe @ 2025-05-07 14:09 UTC (permalink / raw) To: Boris Brezillon Cc: linux-kernel, dri-devel, kernel, Rob Herring, Steven Price, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Sumit Semwal, Christian König, linux-media, linaro-mm-sig On 06.05.2025 09:04, Boris Brezillon wrote: > 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. Done. > > > + /** @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. > }; I think in the case of Panthor we left them outside of a specific structure, but I think it's a good idea. > > 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). Done. > > + > > 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. Deleted. > > + > > + 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__ */ Adrian Larumbe ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-10-07 8:23 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 2025-05-07 14:09 ` Adrián Larumbe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox