On 29/09/2026 04:44, Adrián Larumbe wrote: > Because debugfs knobs are a device interface, and also the different knobs > deal with specific subsystems, it's best to move their initialisations away > from panfrost_drv.c and into the subsystem they provide information about. > > Reviewed-by: Boris Brezillon <[email protected]> > Signed-off-by: Adrián Larumbe <[email protected]> > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 8 ++ > drivers/gpu/drm/panfrost/panfrost_device.h | 4 + > drivers/gpu/drm/panfrost/panfrost_drv.c | 129 > +---------------------------- > drivers/gpu/drm/panfrost/panfrost_gem.c | 29 ++++++- > drivers/gpu/drm/panfrost/panfrost_gem.h | 3 +- > drivers/gpu/drm/panfrost/panfrost_job.c | 99 ++++++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_job.h | 4 + > 7 files changed, 144 insertions(+), 132 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > b/drivers/gpu/drm/panfrost/panfrost_device.c > index c1a2e93a2b9b..5bf528f41278 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -561,3 +561,11 @@ EXPORT_GPL_DEV_PM_OPS(panfrost_pm_ops) = { > RUNTIME_PM_OPS(panfrost_device_runtime_suspend, > panfrost_device_runtime_resume, NULL) > SYSTEM_SLEEP_PM_OPS(panfrost_device_suspend, panfrost_device_resume) > }; > + > +#ifdef CONFIG_DEBUG_FS > +void panfrost_device_debugfs_init(struct drm_minor *minor) > +{ > + panfrost_gem_debugfs_init(minor); > + panfrost_sched_debugfs_init(minor); > +} > +#endif // CONFIG_DEBUG_FS
NIT: I'd usually try to avoid #ifdefs like this by making the header files define a no-op dummy inline function when !CONFIG_DEBUG_FS. It saves polluting the code with a potentially large number of #ifdefs. But I have to admit this isn't too bad and a definite improvement, so it's up to you - either way: Reviewed-by: Steven Price <[email protected]> Thanks, Steve > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h > b/drivers/gpu/drm/panfrost/panfrost_device.h > index 1fe1554f2652..a0b9a2145fc9 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -341,4 +341,8 @@ panfrost_device_schedule_reset(struct panfrost_device > *pfdev) > queue_work(pfdev->reset.wq, &pfdev->reset.work); > } > > +#ifdef CONFIG_DEBUG_FS > +void panfrost_device_debugfs_init(struct drm_minor *minor); > +#endif // CONFIG_DEBUG_FS > + > #endif > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c > b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 59af79a534fa..55fc22e8d4d4 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -792,133 +792,6 @@ 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 panfrost_device *pfdev = to_panfrost_device(node->minor->dev); > - > - panfrost_gem_debugfs_print_bos(pfdev, m); > - > - return 0; > -} > - > -static void show_panfrost_jm_ctx(struct panfrost_jm_ctx *jm_ctx, u32 handle, > - struct seq_file *m) > -{ > - struct drm_device *ddev = ((struct drm_info_node > *)m->private)->minor->dev; > - const char *prio = "UNKNOWN"; > - > - static const char * const prios[] = { > - [DRM_SCHED_PRIORITY_HIGH] = "HIGH", > - [DRM_SCHED_PRIORITY_NORMAL] = "NORMAL", > - [DRM_SCHED_PRIORITY_LOW] = "LOW", > - }; > - > - if (jm_ctx->slot_entity[0].priority != > - jm_ctx->slot_entity[1].priority) > - drm_warn(ddev, "Slot priorities should be the same in a single > context"); > - > - if (jm_ctx->slot_entity[0].priority < ARRAY_SIZE(prios)) > - prio = prios[jm_ctx->slot_entity[0].priority]; > - > - seq_printf(m, " JM context %u: priority %s\n", handle, prio); > -} > - > -static int show_file_jm_ctxs(struct panfrost_file_priv *pfile, > - struct seq_file *m) > -{ > - struct panfrost_jm_ctx *jm_ctx; > - unsigned long i; > - > - xa_lock(&pfile->jm_ctxs); > - xa_for_each(&pfile->jm_ctxs, i, jm_ctx) { > - jm_ctx = panfrost_jm_ctx_get(jm_ctx); > - xa_unlock(&pfile->jm_ctxs); > - show_panfrost_jm_ctx(jm_ctx, i, m); > - panfrost_jm_ctx_put(jm_ctx); > - xa_lock(&pfile->jm_ctxs); > - } > - xa_unlock(&pfile->jm_ctxs); > - > - 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 int show_each_file(struct seq_file *m, void *arg) > -{ > - struct drm_info_node *node = (struct drm_info_node *)m->private; > - struct drm_device *ddev = node->minor->dev; > - int (*show)(struct panfrost_file_priv *, struct seq_file *) = > - node->info_ent->data; > - struct drm_file *file; > - int ret; > - > - ret = mutex_lock_interruptible(&ddev->filelist_mutex); > - if (ret) > - return ret; > - > - list_for_each_entry(file, &ddev->filelist, lhead) { > - struct task_struct *task; > - struct panfrost_file_priv *pfile = file->driver_priv; > - struct pid *pid; > - > - /* > - * Although we have a valid reference on file->pid, that does > - * not guarantee that the task_struct who called get_pid() is > - * still alive (e.g. get_pid(current) => fork() => exit()). > - * Therefore, we need to protect this ->comm access using RCU. > - */ > - rcu_read_lock(); > - pid = rcu_dereference(file->pid); > - task = pid_task(pid, PIDTYPE_TGID); > - seq_printf(m, "client_id %8llu pid %8d command %s:\n", > - file->client_id, pid_nr(pid), > - task ? task->comm : "<unknown>"); > - rcu_read_unlock(); > - > - ret = show(pfile, m); > - if (ret < 0) > - break; > - > - seq_puts(m, "\n"); > - } > - > - mutex_unlock(&ddev->filelist_mutex); > - return ret; > -} > - > -static struct drm_info_list panfrost_sched_debugfs_list[] = { > - { "sched_ctxs", show_each_file, 0, show_file_jm_ctxs }, > -}; > - > -static void panfrost_sched_debugfs_init(struct drm_minor *minor) > -{ > - drm_debugfs_create_files(panfrost_sched_debugfs_list, > - ARRAY_SIZE(panfrost_sched_debugfs_list), > - minor->debugfs_root, minor); > -} > - > -static void panfrost_debugfs_init(struct drm_minor *minor) > -{ > - panthor_gems_debugfs_init(minor); > - panfrost_sched_debugfs_init(minor); > -} > -#endif > - > /* > * Panfrost driver version: > * - 1.0 - initial interface > @@ -950,7 +823,7 @@ static const struct drm_driver panfrost_drm_driver = { > .gem_prime_import = panfrost_gem_prime_import, > .gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table, > #ifdef CONFIG_DEBUG_FS > - .debugfs_init = panfrost_debugfs_init, > + .debugfs_init = panfrost_device_debugfs_init, > #endif > }; > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c > b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 383ac8cd5645..518f24549b9d 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -9,6 +9,7 @@ > #include <linux/dma-mapping.h> > > #include <drm/panfrost_drm.h> > +#include <drm/drm_debugfs.h> > #include <drm/drm_print.h> > #include "panfrost_device.h" > #include "panfrost_gem.h" > @@ -735,8 +736,8 @@ static void panfrost_gem_debugfs_bo_print(struct > panfrost_gem_object *bo, > totals->reclaimable += resident_size; > } > > -void panfrost_gem_debugfs_print_bos(struct panfrost_device *pfdev, > - struct seq_file *m) > +static void panfrost_gem_debugfs_print_bos(struct panfrost_device *pfdev, > + struct seq_file *m) > { > struct gem_size_totals totals = {0}; > struct panfrost_gem_object *bo; > @@ -756,4 +757,28 @@ void panfrost_gem_debugfs_print_bos(struct > panfrost_device *pfdev, > seq_printf(m, "Total size: %zd, Total resident: %zd, Total reclaimable: > %zd\n", > totals.size, totals.resident, totals.reclaimable); > } > + > +static int panfrost_gems_show(struct seq_file *m, void *data) > +{ > + struct drm_info_node *node = m->private; > + struct panfrost_device *pfdev = to_panfrost_device(node->minor->dev); > + > + panfrost_gem_debugfs_print_bos(pfdev, m); > + > + return 0; > +} > + > +static struct drm_info_list panfrost_debugfs_list[] = { > + {"gems", > + panfrost_gems_show, 0, NULL}, > +}; > + > +int panfrost_gem_debugfs_init(struct drm_minor *minor) > +{ > + drm_debugfs_create_files(panfrost_debugfs_list, > + ARRAY_SIZE(panfrost_debugfs_list), > + minor->debugfs_root, minor); > + > + return 0; > +} > #endif > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h > b/drivers/gpu/drm/panfrost/panfrost_gem.h > index 5c823cdbd980..3008afcf4bff 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -164,8 +164,7 @@ int panfrost_gem_sync(struct drm_gem_object *obj, u32 > type, > void panfrost_gem_internal_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); > +int panfrost_gem_debugfs_init(struct drm_minor *minor); > #endif > > #endif /* __PANFROST_GEM_H__ */ > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c > b/drivers/gpu/drm/panfrost/panfrost_job.c > index 476c936b54e6..00fb36c577df 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -8,6 +8,8 @@ > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/dma-resv.h> > +#include <drm/drm_debugfs.h> > +#include <drm/drm_print.h> > #include <drm/gpu_scheduler.h> > #include <drm/panfrost_drm.h> > > @@ -1138,3 +1140,100 @@ int panfrost_jm_ctx_destroy(struct drm_file *file, > u32 handle) > panfrost_jm_ctx_put(jm_ctx); > return 0; > } > + > +#ifdef CONFIG_DEBUG_FS > +static void show_panfrost_jm_ctx(struct panfrost_jm_ctx *jm_ctx, u32 handle, > + struct seq_file *m) > +{ > + struct drm_device *ddev = ((struct drm_info_node > *)m->private)->minor->dev; > + const char *prio = "UNKNOWN"; > + > + static const char * const prios[] = { > + [DRM_SCHED_PRIORITY_HIGH] = "HIGH", > + [DRM_SCHED_PRIORITY_NORMAL] = "NORMAL", > + [DRM_SCHED_PRIORITY_LOW] = "LOW", > + }; > + > + if (jm_ctx->slot_entity[0].priority != > + jm_ctx->slot_entity[1].priority) > + drm_warn(ddev, "Slot priorities should be the same in a single > context"); > + > + if (jm_ctx->slot_entity[0].priority < ARRAY_SIZE(prios)) > + prio = prios[jm_ctx->slot_entity[0].priority]; > + > + seq_printf(m, " JM context %u: priority %s\n", handle, prio); > +} > + > +static int show_file_jm_ctxs(struct panfrost_file_priv *pfile, > + struct seq_file *m) > +{ > + struct panfrost_jm_ctx *jm_ctx; > + unsigned long i; > + > + xa_lock(&pfile->jm_ctxs); > + xa_for_each(&pfile->jm_ctxs, i, jm_ctx) { > + jm_ctx = panfrost_jm_ctx_get(jm_ctx); > + xa_unlock(&pfile->jm_ctxs); > + show_panfrost_jm_ctx(jm_ctx, i, m); > + panfrost_jm_ctx_put(jm_ctx); > + xa_lock(&pfile->jm_ctxs); > + } > + xa_unlock(&pfile->jm_ctxs); > + > + return 0; > +} > + > +static int show_each_file(struct seq_file *m, void *arg) > +{ > + struct drm_info_node *node = (struct drm_info_node *)m->private; > + struct drm_device *ddev = node->minor->dev; > + int (*show)(struct panfrost_file_priv *, struct seq_file *) = > + node->info_ent->data; > + struct drm_file *file; > + int ret; > + > + ret = mutex_lock_interruptible(&ddev->filelist_mutex); > + if (ret) > + return ret; > + > + list_for_each_entry(file, &ddev->filelist, lhead) { > + struct task_struct *task; > + struct panfrost_file_priv *pfile = file->driver_priv; > + struct pid *pid; > + > + /* > + * Although we have a valid reference on file->pid, that does > + * not guarantee that the task_struct who called get_pid() is > + * still alive (e.g. get_pid(current) => fork() => exit()). > + * Therefore, we need to protect this ->comm access using RCU. > + */ > + rcu_read_lock(); > + pid = rcu_dereference(file->pid); > + task = pid_task(pid, PIDTYPE_TGID); > + seq_printf(m, "client_id %8llu pid %8d command %s:\n", > + file->client_id, pid_nr(pid), > + task ? task->comm : "<unknown>"); > + rcu_read_unlock(); > + > + ret = show(pfile, m); > + if (ret < 0) > + break; > + > + seq_puts(m, "\n"); > + } > + > + mutex_unlock(&ddev->filelist_mutex); > + return ret; > +} > + > +static struct drm_info_list panfrost_sched_debugfs_list[] = { > + { "sched_ctxs", show_each_file, 0, show_file_jm_ctxs }, > +}; > + > +void panfrost_sched_debugfs_init(struct drm_minor *minor) > +{ > + drm_debugfs_create_files(panfrost_sched_debugfs_list, > + ARRAY_SIZE(panfrost_sched_debugfs_list), > + minor->debugfs_root, minor); > +} > +#endif > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.h > b/drivers/gpu/drm/panfrost/panfrost_job.h > index c3f57e41a571..3e3b9717838f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.h > +++ b/drivers/gpu/drm/panfrost/panfrost_job.h > @@ -74,4 +74,8 @@ int panfrost_job_get_slot(struct panfrost_job *job); > int panfrost_job_push(struct panfrost_job *job); > void panfrost_job_put(struct panfrost_job *job); > > +#ifdef CONFIG_DEBUG_FS > +void panfrost_sched_debugfs_init(struct drm_minor *minor); > +#endif > + > #endif >
