[PATCH] drm/accel/amdxdna: add debugfs support

[PATCH] drm/accel/amdxdna: add debugfs support

[Md Shofiqul Islam]

Expose driver state through debugfs to aid debugging and development.
Three read-only entries are created under /sys/kernel/debug/accel/<N>/:

 - fw_version:  firmware version as major.minor.sub.build
 - device_info: device VBNV string and device type identifier
 - clients:     per-fd info (pid, heap usage, total BO usage)

Resolves the TODO item in drivers/accel/amdxdna/TODO.

Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
 drivers/accel/amdxdna/Makefile          |  1 +
 drivers/accel/amdxdna/amdxdna_debugfs.c | 63 +++++++++++++++++++++++++
 drivers/accel/amdxdna/amdxdna_debugfs.h | 17 +++++++
 drivers/accel/amdxdna/amdxdna_pci_drv.c |  3 ++
 4 files changed, 84 insertions(+)
 create mode 100644 drivers/accel/amdxdna/amdxdna_debugfs.c
 create mode 100644 drivers/accel/amdxdna/amdxdna_debugfs.h

diff --git a/drivers/accel/amdxdna/Makefile b/drivers/accel/amdxdna/Makefile
index cf9bf19de..01a809d88 100644
--- a/drivers/accel/amdxdna/Makefile
+++ b/drivers/accel/amdxdna/Makefile
@@ -10,6 +10,7 @@ amdxdna-y := \
 	aie2_smu.o \
 	aie2_solver.o \
 	amdxdna_ctx.o \
+	amdxdna_debugfs.o \
 	amdxdna_gem.o \
 	amdxdna_iommu.o \
 	amdxdna_mailbox.o \
diff --git a/drivers/accel/amdxdna/amdxdna_debugfs.c b/drivers/accel/amdxdna/amdxdna_debugfs.c
new file mode 100644
index 000000000..6806698c3
--- /dev/null
+++ b/drivers/accel/amdxdna/amdxdna_debugfs.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024, Advanced Micro Devices, Inc.
+ */
+
+#include <drm/drm_debugfs.h>
+#include <drm/drm_device.h>
+
+#include "amdxdna_debugfs.h"
+#include "amdxdna_pci_drv.h"
+
+static inline struct amdxdna_dev *seq_to_xdna(struct seq_file *s)
+{
+	struct drm_debugfs_entry *entry = s->private;
+
+	return to_xdna_dev(entry->dev);
+}
+
+static int fw_version_show(struct seq_file *s, void *v)
+{
+	struct amdxdna_dev *xdna = seq_to_xdna(s);
+
+	seq_printf(s, "%d.%d.%d.%d\n",
+		   xdna->fw_ver.major, xdna->fw_ver.minor,
+		   xdna->fw_ver.sub, xdna->fw_ver.build);
+	return 0;
+}
+
+static int device_info_show(struct seq_file *s, void *v)
+{
+	struct amdxdna_dev *xdna = seq_to_xdna(s);
+
+	seq_printf(s, "vbnv:        %s\n", xdna->dev_info->vbnv);
+	seq_printf(s, "device_type: %d\n", xdna->dev_info->device_type);
+	return 0;
+}
+
+static int clients_show(struct seq_file *s, void *v)
+{
+	struct amdxdna_dev *xdna = seq_to_xdna(s);
+	struct amdxdna_client *client;
+
+	mutex_lock(&xdna->dev_lock);
+	list_for_each_entry(client, &xdna->client_list, node)
+		seq_printf(s, "pid %d heap_usage %zu total_bo_usage %zu\n",
+			   client->pid, client->heap_usage,
+			   client->total_bo_usage);
+	mutex_unlock(&xdna->dev_lock);
+
+	return 0;
+}
+
+static const struct drm_debugfs_info amdxdna_debugfs_list[] = {
+	{ "fw_version",  fw_version_show,  0 },
+	{ "device_info", device_info_show, 0 },
+	{ "clients",     clients_show,     0 },
+};
+
+void amdxdna_debugfs_init(struct amdxdna_dev *xdna)
+{
+	drm_debugfs_add_files(&xdna->ddev, amdxdna_debugfs_list,
+			      ARRAY_SIZE(amdxdna_debugfs_list));
+}
diff --git a/drivers/accel/amdxdna/amdxdna_debugfs.h b/drivers/accel/amdxdna/amdxdna_debugfs.h
new file mode 100644
index 000000000..e742a81fe
--- /dev/null
+++ b/drivers/accel/amdxdna/amdxdna_debugfs.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2024, Advanced Micro Devices, Inc.
+ */
+
+#ifndef _AMDXDNA_DEBUGFS_H_
+#define _AMDXDNA_DEBUGFS_H_
+
+struct amdxdna_dev;
+
+#ifdef CONFIG_DEBUG_FS
+void amdxdna_debugfs_init(struct amdxdna_dev *xdna);
+#else
+static inline void amdxdna_debugfs_init(struct amdxdna_dev *xdna) {}
+#endif
+
+#endif /* _AMDXDNA_DEBUGFS_H_ */
diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
index b50a7d1f8..19866dbed 100644
--- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
+++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
@@ -15,6 +15,7 @@
 #include <linux/pci.h>
 
 #include "amdxdna_ctx.h"
+#include "amdxdna_debugfs.h"
 #include "amdxdna_gem.h"
 #include "amdxdna_pci_drv.h"
 #include "amdxdna_pm.h"
@@ -319,6 +320,8 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto failed_sysfs_fini;
 	}
 
+	amdxdna_debugfs_init(xdna);
+
 	return 0;
 
 failed_sysfs_fini:
-- 
2.51.1

Re: [PATCH] drm/accel/amdxdna: add debugfs support

[Lizhi Hou]

On 5/19/26 13:32, Md Shofiqul Islam wrote:
> Expose driver state through debugfs to aid debugging and development.
> Three read-only entries are created under /sys/kernel/debug/accel/<N>/:
>
>   - fw_version:  firmware version as major.minor.sub.build
>   - device_info: device VBNV string and device type identifier
These are provided by sysfs already. (amdxdna_sysfs.c)
>   - clients:     per-fd info (pid, heap usage, total BO usage)

This is also supported by 
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/e0169d0c690f03f379bf71d9332bb2065a5c6a05


Thanks,

Lizhi

>
> Resolves the TODO item in drivers/accel/amdxdna/TODO.
>
> Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
> ---
>   drivers/accel/amdxdna/Makefile          |  1 +
>   drivers/accel/amdxdna/amdxdna_debugfs.c | 63 +++++++++++++++++++++++++
>   drivers/accel/amdxdna/amdxdna_debugfs.h | 17 +++++++
>   drivers/accel/amdxdna/amdxdna_pci_drv.c |  3 ++
>   4 files changed, 84 insertions(+)
>   create mode 100644 drivers/accel/amdxdna/amdxdna_debugfs.c
>   create mode 100644 drivers/accel/amdxdna/amdxdna_debugfs.h
>
> diff --git a/drivers/accel/amdxdna/Makefile b/drivers/accel/amdxdna/Makefile
> index cf9bf19de..01a809d88 100644
> --- a/drivers/accel/amdxdna/Makefile
> +++ b/drivers/accel/amdxdna/Makefile
> @@ -10,6 +10,7 @@ amdxdna-y := \
>   	aie2_smu.o \
>   	aie2_solver.o \
>   	amdxdna_ctx.o \
> +	amdxdna_debugfs.o \
>   	amdxdna_gem.o \
>   	amdxdna_iommu.o \
>   	amdxdna_mailbox.o \
> diff --git a/drivers/accel/amdxdna/amdxdna_debugfs.c b/drivers/accel/amdxdna/amdxdna_debugfs.c
> new file mode 100644
> index 000000000..6806698c3
> --- /dev/null
> +++ b/drivers/accel/amdxdna/amdxdna_debugfs.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024, Advanced Micro Devices, Inc.
> + */
> +
> +#include <drm/drm_debugfs.h>
> +#include <drm/drm_device.h>
> +
> +#include "amdxdna_debugfs.h"
> +#include "amdxdna_pci_drv.h"
> +
> +static inline struct amdxdna_dev *seq_to_xdna(struct seq_file *s)
> +{
> +	struct drm_debugfs_entry *entry = s->private;
> +
> +	return to_xdna_dev(entry->dev);
> +}
> +
> +static int fw_version_show(struct seq_file *s, void *v)
> +{
> +	struct amdxdna_dev *xdna = seq_to_xdna(s);
> +
> +	seq_printf(s, "%d.%d.%d.%d\n",
> +		   xdna->fw_ver.major, xdna->fw_ver.minor,
> +		   xdna->fw_ver.sub, xdna->fw_ver.build);
> +	return 0;
> +}
> +
> +static int device_info_show(struct seq_file *s, void *v)
> +{
> +	struct amdxdna_dev *xdna = seq_to_xdna(s);
> +
> +	seq_printf(s, "vbnv:        %s\n", xdna->dev_info->vbnv);
> +	seq_printf(s, "device_type: %d\n", xdna->dev_info->device_type);
> +	return 0;
> +}
> +
> +static int clients_show(struct seq_file *s, void *v)
> +{
> +	struct amdxdna_dev *xdna = seq_to_xdna(s);
> +	struct amdxdna_client *client;
> +
> +	mutex_lock(&xdna->dev_lock);
> +	list_for_each_entry(client, &xdna->client_list, node)
> +		seq_printf(s, "pid %d heap_usage %zu total_bo_usage %zu\n",
> +			   client->pid, client->heap_usage,
> +			   client->total_bo_usage);
> +	mutex_unlock(&xdna->dev_lock);
> +
> +	return 0;
> +}
> +
> +static const struct drm_debugfs_info amdxdna_debugfs_list[] = {
> +	{ "fw_version",  fw_version_show,  0 },
> +	{ "device_info", device_info_show, 0 },
> +	{ "clients",     clients_show,     0 },
> +};
> +
> +void amdxdna_debugfs_init(struct amdxdna_dev *xdna)
> +{
> +	drm_debugfs_add_files(&xdna->ddev, amdxdna_debugfs_list,
> +			      ARRAY_SIZE(amdxdna_debugfs_list));
> +}
> diff --git a/drivers/accel/amdxdna/amdxdna_debugfs.h b/drivers/accel/amdxdna/amdxdna_debugfs.h
> new file mode 100644
> index 000000000..e742a81fe
> --- /dev/null
> +++ b/drivers/accel/amdxdna/amdxdna_debugfs.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2024, Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _AMDXDNA_DEBUGFS_H_
> +#define _AMDXDNA_DEBUGFS_H_
> +
> +struct amdxdna_dev;
> +
> +#ifdef CONFIG_DEBUG_FS
> +void amdxdna_debugfs_init(struct amdxdna_dev *xdna);
> +#else
> +static inline void amdxdna_debugfs_init(struct amdxdna_dev *xdna) {}
> +#endif
> +
> +#endif /* _AMDXDNA_DEBUGFS_H_ */
> diff --git a/drivers/accel/amdxdna/amdxdna_pci_drv.c b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> index b50a7d1f8..19866dbed 100644
> --- a/drivers/accel/amdxdna/amdxdna_pci_drv.c
> +++ b/drivers/accel/amdxdna/amdxdna_pci_drv.c
> @@ -15,6 +15,7 @@
>   #include <linux/pci.h>
>   
>   #include "amdxdna_ctx.h"
> +#include "amdxdna_debugfs.h"
>   #include "amdxdna_gem.h"
>   #include "amdxdna_pci_drv.h"
>   #include "amdxdna_pm.h"
> @@ -319,6 +320,8 @@ static int amdxdna_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   		goto failed_sysfs_fini;
>   	}
>   
> +	amdxdna_debugfs_init(xdna);
> +
>   	return 0;
>   
>   failed_sysfs_fini:

Claude review: drm/accel/amdxdna: add debugfs support

[Claude Code Review Bot]

Overall Series Review

Subject: drm/accel/amdxdna: add debugfs support
Author: Md Shofiqul Islam <shofiqtest@gmail.com>
Patches: 2
Reviewed: 2026-05-25T22:09:05.249602

---

**NAK.** This patch is based on a significantly older version of the amdxdna driver and is superseded by work already merged. The current drm-next tree already contains:

1. A fully functional `amdxdna_debugfs.c` (with a carveout debugfs entry)
2. `amdxdna_debugfs.h` with proper `CONFIG_DEBUG_FS` guards
3. The `amdxdna_debugfs_init()` call in `amdxdna_probe()`
4. Conditional Makefile inclusion via `amdxdna-$(CONFIG_DEBUG_FS)`

Beyond the conflict with existing code, the patch contains a compilation error and a Makefile correctness issue. The TODO item referenced in the commit message has effectively already been addressed.

---

---
Generated by Claude Code Patch Reviewer

Claude review: drm/accel/amdxdna: add debugfs support

[Claude Code Review Bot]

Patch Review

**1. Superseded by existing implementation (fatal)**

The current tree already has `drivers/accel/amdxdna/amdxdna_debugfs.c` with a working debugfs implementation using `debugfs_create_file()`. The patch attempts to create this file from scratch, which would discard the existing carveout debugfs support. The existing code also already calls `amdxdna_debugfs_init()` from `amdxdna_pci_drv.c` and includes the header.

**2. Wrong field access — `xdna->dev_info->vbnv` does not exist (compilation error)**

```c
+	seq_printf(s, "vbnv:        %s\n", xdna->dev_info->vbnv);
```

`struct amdxdna_dev_info` has `default_vbnv` (at `amdxdna_pci_drv.h:96`), not `vbnv`. The resolved vbnv string is stored directly on the device struct at `xdna->vbnv` (`amdxdna_pci_drv.h:128`). This would fail to compile. The existing `amdxdna_sysfs.c` shows the correct access pattern:

```c
// amdxdna_sysfs.c:23
return sprintf(buf, "%s\n", xdna->vbnv);
```

**3. Unconditional Makefile addition breaks `!CONFIG_DEBUG_FS` builds**

```diff
+	amdxdna_debugfs.o \
```

The patch adds `amdxdna_debugfs.o` unconditionally to `amdxdna-y`. The current tree correctly uses:
```makefile
amdxdna-$(CONFIG_DEBUG_FS) += amdxdna_debugfs.o
```

While the header provides a `static inline` stub when `CONFIG_DEBUG_FS` is disabled, the `.c` file includes `<drm/drm_debugfs.h>` and calls `drm_debugfs_add_files()`, which won't be available in `!CONFIG_DEBUG_FS` builds. The Makefile should use the conditional form.

**4. Uses `drm_debugfs_add_files()` / `drm_debugfs_info` instead of the driver's existing pattern**

```c
+static const struct drm_debugfs_info amdxdna_debugfs_list[] = {
+	{ "fw_version",  fw_version_show,  0 },
+	{ "device_info", device_info_show, 0 },
+	{ "clients",     clients_show,     0 },
+};
+
+void amdxdna_debugfs_init(struct amdxdna_dev *xdna)
+{
+	drm_debugfs_add_files(&xdna->ddev, amdxdna_debugfs_list,
+			      ARRAY_SIZE(amdxdna_debugfs_list));
+}
```

The existing code uses `debugfs_create_file()` with custom `file_operations` (via the `AMDXDNA_DBGFS_FOPS` macro), passing `xdna` as `inode->i_private`. The DRM helpers approach passes `entry->dev` through `seq_to_xdna()`. If new entries were to be added, they should follow the existing driver pattern for consistency. The DRM helper approach is cleaner for simple read-only entries, but mixing two different debugfs paradigms in one driver would be confusing.

**5. Copyright year is stale**

```c
+/*
+ * Copyright (C) 2024, Advanced Micro Devices, Inc.
+ */
```

The existing file in the tree uses `Copyright (C) 2026`. New code should use the current year.

**6. Missing NULL check on `xdna->vbnv` (minor, even if field were corrected)**

The existing `amdxdna_sysfs.c:20` checks `if (!xdna->vbnv)` before printing. The debugfs `device_info_show` doesn't, which could print `(null)` if called before firmware initialization populates the vbnv string.

**Recommendation:** If the intent is to add fw_version, device_info, and clients debugfs entries, this should be done as additions to the existing `amdxdna_debugfs.c`, following the established `AMDXDNA_DBGFS_FOPS` / `debugfs_create_file()` pattern, and with the correct field accesses. The patch in its current form cannot be applied.

---
Generated by Claude Code Patch Reviewer