From: Rob Clark <[email protected]>
Similar motivation to other similar recent attempt[1]. But with an
attempt to have some shared code for this. As well as documentation.
It is probably a bit UMA-centric, I guess devices with VRAM might want
some placement stats as well. But this seems like a reasonable start.
Basic gputop support: https://patchwork.freedesktop.org/series/116236/
And already nvtop support: https://github.com/Syllo/nvtop/pull/204
[1] https://patchwork.freedesktop.org/series/112397/
Rob Clark (6):
drm: Add common fdinfo helper
drm/msm: Switch to fdinfo helper
drm/amdgpu: Switch to fdinfo helper
drm/i915: Switch to fdinfo helper
drm: Add fdinfo memory stats
drm/msm: Add memory stats to fdinfo
Documentation/gpu/drm-usage-stats.rst | 31 +++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 16 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h | 2 +-
drivers/gpu/drm/drm_file.c | 111 +++++++++++++++++++++
drivers/gpu/drm/i915/i915_driver.c | 3 +-
drivers/gpu/drm/i915/i915_drm_client.c | 18 +---
drivers/gpu/drm/i915/i915_drm_client.h | 2 +-
drivers/gpu/drm/msm/msm_drv.c | 11 +-
drivers/gpu/drm/msm/msm_gem.c | 15 +++
drivers/gpu/drm/msm/msm_gpu.c | 2 -
include/drm/drm_drv.h | 7 ++
include/drm/drm_file.h | 5 +
include/drm/drm_gem.h | 30 ++++++
14 files changed, 220 insertions(+), 36 deletions(-)
--
2.39.2
From: Rob Clark <[email protected]>
Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/i915/i915_driver.c | 3 ++-
drivers/gpu/drm/i915/i915_drm_client.c | 18 +++++-------------
drivers/gpu/drm/i915/i915_drm_client.h | 2 +-
3 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index db7a86def7e2..0d91f85f8b97 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1696,7 +1696,7 @@ static const struct file_operations i915_driver_fops = {
.compat_ioctl = i915_ioc32_compat_ioctl,
.llseek = noop_llseek,
#ifdef CONFIG_PROC_FS
- .show_fdinfo = i915_drm_client_fdinfo,
+ .show_fdinfo = drm_show_fdinfo,
#endif
};
@@ -1796,6 +1796,7 @@ static const struct drm_driver i915_drm_driver = {
.open = i915_driver_open,
.lastclose = i915_driver_lastclose,
.postclose = i915_driver_postclose,
+ .show_fdinfo = i915_drm_client_fdinfo,
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index b09d1d386574..4a77e5e47f79 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -101,7 +101,7 @@ static u64 busy_add(struct i915_gem_context *ctx, unsigned int class)
}
static void
-show_client_class(struct seq_file *m,
+show_client_class(struct drm_printer *p,
struct i915_drm_client *client,
unsigned int class)
{
@@ -117,22 +117,20 @@ show_client_class(struct seq_file *m,
rcu_read_unlock();
if (capacity)
- seq_printf(m, "drm-engine-%s:\t%llu ns\n",
+ drm_printf(p, "drm-engine-%s:\t%llu ns\n",
uabi_class_names[class], total);
if (capacity > 1)
- seq_printf(m, "drm-engine-capacity-%s:\t%u\n",
+ drm_printf(p, "drm-engine-capacity-%s:\t%u\n",
uabi_class_names[class],
capacity);
}
-void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
+void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file)
{
- struct drm_file *file = f->private_data;
struct drm_i915_file_private *file_priv = file->driver_priv;
struct drm_i915_private *i915 = file_priv->dev_priv;
struct i915_drm_client *client = file_priv->client;
- struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
unsigned int i;
/*
@@ -141,12 +139,6 @@ void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
* ******************************************************************
*/
- seq_printf(m, "drm-driver:\t%s\n", i915->drm.driver->name);
- seq_printf(m, "drm-pdev:\t%04x:%02x:%02x.%d\n",
- pci_domain_nr(pdev->bus), pdev->bus->number,
- PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
- seq_printf(m, "drm-client-id:\t%u\n", client->id);
-
/*
* Temporarily skip showing client engine information with GuC submission till
* fetching engine busyness is implemented in the GuC submission backend
@@ -155,6 +147,6 @@ void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
return;
for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++)
- show_client_class(m, client, i);
+ show_client_class(p, client, i);
}
#endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 69496af996d9..ef85fef45de5 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -60,7 +60,7 @@ static inline void i915_drm_client_put(struct i915_drm_client *client)
struct i915_drm_client *i915_drm_client_add(struct i915_drm_clients *clients);
#ifdef CONFIG_PROC_FS
-void i915_drm_client_fdinfo(struct seq_file *m, struct file *f);
+void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
#endif
void i915_drm_clients_fini(struct i915_drm_clients *clients);
--
2.39.2
From: Rob Clark <[email protected]>
Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 16 ++++++----------
drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h | 2 +-
3 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index f5ffca24def4..6c0e0c614b94 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2752,7 +2752,7 @@ static const struct file_operations amdgpu_driver_kms_fops = {
.compat_ioctl = amdgpu_kms_compat_ioctl,
#endif
#ifdef CONFIG_PROC_FS
- .show_fdinfo = amdgpu_show_fdinfo
+ .show_fdinfo = drm_show_fdinfo,
#endif
};
@@ -2807,6 +2807,7 @@ static const struct drm_driver amdgpu_kms_driver = {
.dumb_map_offset = amdgpu_mode_dumb_mmap,
.fops = &amdgpu_driver_kms_fops,
.release = &amdgpu_driver_release_kms,
+ .show_fdinfo = amdgpu_show_fdinfo,
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
index 99a7855ab1bc..c2fdd5e448d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
@@ -53,9 +53,8 @@ static const char *amdgpu_ip_name[AMDGPU_HW_IP_NUM] = {
[AMDGPU_HW_IP_VCN_JPEG] = "jpeg",
};
-void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
+void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
{
- struct drm_file *file = f->private_data;
struct amdgpu_device *adev = drm_to_adev(file->minor->dev);
struct amdgpu_fpriv *fpriv = file->driver_priv;
struct amdgpu_vm *vm = &fpriv->vm;
@@ -86,18 +85,15 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
* ******************************************************************
*/
- seq_printf(m, "pasid:\t%u\n", fpriv->vm.pasid);
- seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name);
- seq_printf(m, "drm-pdev:\t%04x:%02x:%02x.%d\n", domain, bus, dev, fn);
- seq_printf(m, "drm-client-id:\t%Lu\n", vm->immediate.fence_context);
- seq_printf(m, "drm-memory-vram:\t%llu KiB\n", vram_mem/1024UL);
- seq_printf(m, "drm-memory-gtt: \t%llu KiB\n", gtt_mem/1024UL);
- seq_printf(m, "drm-memory-cpu: \t%llu KiB\n", cpu_mem/1024UL);
+ drm_printf(p, "pasid:\t%u\n", fpriv->vm.pasid);
+ drm_printf(p, "drm-memory-vram:\t%llu KiB\n", vram_mem/1024UL);
+ drm_printf(p, "drm-memory-gtt: \t%llu KiB\n", gtt_mem/1024UL);
+ drm_printf(p, "drm-memory-cpu: \t%llu KiB\n", cpu_mem/1024UL);
for (hw_ip = 0; hw_ip < AMDGPU_HW_IP_NUM; ++hw_ip) {
if (!usage[hw_ip])
continue;
- seq_printf(m, "drm-engine-%s:\t%Ld ns\n", amdgpu_ip_name[hw_ip],
+ drm_printf(p, "drm-engine-%s:\t%Ld ns\n", amdgpu_ip_name[hw_ip],
ktime_to_ns(usage[hw_ip]));
}
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h
index e86834bfea1d..0398f5a159ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h
@@ -37,6 +37,6 @@
#include "amdgpu_ids.h"
uint32_t amdgpu_get_ip_count(struct amdgpu_device *adev, int id);
-void amdgpu_show_fdinfo(struct seq_file *m, struct file *f);
+void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file);
#endif
--
2.39.2
From: Rob Clark <[email protected]>
Use the new helper to export stats about memory usage.
v2: Drop unintended hunk
v3: Rebase
Signed-off-by: Rob Clark <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
---
drivers/gpu/drm/msm/msm_gem.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index db6c4e281d75..c32264234ea1 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -1096,6 +1096,20 @@ int msm_gem_new_handle(struct drm_device *dev, struct drm_file *file,
return ret;
}
+static enum drm_gem_object_status msm_gem_status(struct drm_gem_object *obj)
+{
+ struct msm_gem_object *msm_obj = to_msm_bo(obj);
+ enum drm_gem_object_status status = 0;
+
+ if (msm_obj->pages)
+ status |= DRM_GEM_OBJECT_RESIDENT;
+
+ if (msm_obj->madv == MSM_MADV_DONTNEED)
+ status |= DRM_GEM_OBJECT_PURGEABLE;
+
+ return status;
+}
+
static const struct vm_operations_struct vm_ops = {
.fault = msm_gem_fault,
.open = drm_gem_vm_open,
@@ -1110,6 +1124,7 @@ static const struct drm_gem_object_funcs msm_gem_object_funcs = {
.vmap = msm_gem_prime_vmap,
.vunmap = msm_gem_prime_vunmap,
.mmap = msm_gem_object_mmap,
+ .status = msm_gem_status,
.vm_ops = &vm_ops,
};
--
2.39.2
From: Rob Clark <[email protected]>
Add support to dump GEM stats to fdinfo.
v2: Fix typos, change size units to match docs, use div_u64
v3: Do it in core
v4: more kerneldoc
Signed-off-by: Rob Clark <[email protected]>
Reviewed-by: Emil Velikov <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
---
Documentation/gpu/drm-usage-stats.rst | 21 ++++++++
drivers/gpu/drm/drm_file.c | 76 +++++++++++++++++++++++++++
include/drm/drm_file.h | 1 +
include/drm/drm_gem.h | 30 +++++++++++
4 files changed, 128 insertions(+)
diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
index 2ab32c40e93c..80003e27e28e 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -105,6 +105,27 @@ object belong to this client, in the respective memory region.
Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB'
indicating kibi- or mebi-bytes.
+- drm-shared-memory: <uint> [KiB|MiB]
+
+The total size of buffers that are shared with another file (ie. have more
+than a single handle).
+
+- drm-private-memory: <uint> [KiB|MiB]
+
+The total size of buffers that are not shared with another file.
+
+- drm-resident-memory: <uint> [KiB|MiB]
+
+The total size of buffers that are resident in system memory.
+
+- drm-purgeable-memory: <uint> [KiB|MiB]
+
+The total size of buffers that are purgeable.
+
+- drm-active-memory: <uint> [KiB|MiB]
+
+The total size of buffers that are active on one or more rings.
+
- drm-cycles-<str> <uint>
Engine identifier string must be the same as the one specified in the
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 6d5bdd684ae2..04a7ed7eba8e 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -42,6 +42,7 @@
#include <drm/drm_client.h>
#include <drm/drm_drv.h>
#include <drm/drm_file.h>
+#include <drm/drm_gem.h>
#include <drm/drm_print.h>
#include "drm_crtc_internal.h"
@@ -871,6 +872,79 @@ void drm_send_event(struct drm_device *dev, struct drm_pending_event *e)
}
EXPORT_SYMBOL(drm_send_event);
+static void print_size(struct drm_printer *p, const char *stat, size_t sz)
+{
+ const char *units[] = {"", " KiB", " MiB"};
+ unsigned u;
+
+ for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
+ if (sz < SZ_1K)
+ break;
+ sz = div_u64(sz, SZ_1K);
+ }
+
+ drm_printf(p, "%s:\t%zu%s\n", stat, sz, units[u]);
+}
+
+static void print_memory_stats(struct drm_printer *p, struct drm_file *file)
+{
+ struct drm_gem_object *obj;
+ struct {
+ size_t shared;
+ size_t private;
+ size_t resident;
+ size_t purgeable;
+ size_t active;
+ } size = {0};
+ bool has_status = false;
+ int id;
+
+ spin_lock(&file->table_lock);
+ idr_for_each_entry (&file->object_idr, obj, id) {
+ enum drm_gem_object_status s = 0;
+
+ if (obj->funcs && obj->funcs->status) {
+ s = obj->funcs->status(obj);
+ has_status = true;
+ }
+
+ if (obj->handle_count > 1) {
+ size.shared += obj->size;
+ } else {
+ size.private += obj->size;
+ }
+
+ if (s & DRM_GEM_OBJECT_RESIDENT) {
+ size.resident += obj->size;
+ } else {
+ /* If already purged or not yet backed by pages, don't
+ * count it as purgeable:
+ */
+ s &= ~DRM_GEM_OBJECT_PURGEABLE;
+ }
+
+ if (!dma_resv_test_signaled(obj->resv, dma_resv_usage_rw(true))) {
+ size.active += obj->size;
+
+ /* If still active, don't count as purgeable: */
+ s &= ~DRM_GEM_OBJECT_PURGEABLE;
+ }
+
+ if (s & DRM_GEM_OBJECT_PURGEABLE)
+ size.purgeable += obj->size;
+ }
+ spin_unlock(&file->table_lock);
+
+ print_size(p, "drm-shared-memory", size.shared);
+ print_size(p, "drm-private-memory", size.private);
+ print_size(p, "drm-active-memory", size.active);
+
+ if (has_status) {
+ print_size(p, "drm-resident-memory", size.resident);
+ print_size(p, "drm-purgeable-memory", size.purgeable);
+ }
+}
+
/**
* drm_show_fdinfo - helper for drm file fops
* @seq_file: output stream
@@ -900,6 +974,8 @@ void drm_show_fdinfo(struct seq_file *m, struct file *f)
if (dev->driver->show_fdinfo)
dev->driver->show_fdinfo(&p, file);
+
+ print_memory_stats(&p, file);
}
EXPORT_SYMBOL(drm_show_fdinfo);
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 6de6d0e9c634..919284bb4f1d 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -41,6 +41,7 @@
struct dma_fence;
struct drm_file;
struct drm_device;
+struct drm_printer;
struct device;
struct file;
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 189fd618ca65..9ebd2820ad1f 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -42,6 +42,25 @@
struct iosys_map;
struct drm_gem_object;
+/**
+ * enum drm_gem_object_status - bitmask of object state for fdinfo reporting
+ * @DRM_GEM_OBJECT_RESIDENT: object is resident in memory (ie. not unpinned)
+ * @DRM_GEM_OBJECT_PURGEABLE: object marked as purgeable by userspace
+ *
+ * Bitmask of status used for fdinfo memory stats, see &drm_gem_object_funcs.status
+ * and drm_show_fdinfo(). Note that an object can DRM_GEM_OBJECT_PURGEABLE if
+ * it still active or not resident, in which case drm_show_fdinfo() will not
+ * account for it as purgeable. So drivers do not need to check if the buffer
+ * is idle and resident to return this bit. (Ie. userspace can mark a buffer
+ * as purgeable even while it is still busy on the GPU.. it does not _actually_
+ * become puregeable until it becomes idle. The status gem object func does
+ * not need to consider this.)
+ */
+enum drm_gem_object_status {
+ DRM_GEM_OBJECT_RESIDENT = BIT(0),
+ DRM_GEM_OBJECT_PURGEABLE = BIT(1),
+};
+
/**
* struct drm_gem_object_funcs - GEM object functions
*/
@@ -174,6 +193,17 @@ struct drm_gem_object_funcs {
*/
int (*evict)(struct drm_gem_object *obj);
+ /**
+ * @status:
+ *
+ * The optional status callback can return additional object state
+ * which determines which stats the object is counted against. The
+ * callback is called under table_lock. Racing against object status
+ * change is "harmless", and the callback can expect to not race
+ * against object destruction.
+ */
+ enum drm_gem_object_status (*status)(struct drm_gem_object *obj);
+
/**
* @vm_ops:
*
--
2.39.2
Am 13.04.23 um 00:42 schrieb Rob Clark:
> From: Rob Clark <[email protected]>
>
> Signed-off-by: Rob Clark <[email protected]>
Reviewed-by: Christian König <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 16 ++++++----------
> drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h | 2 +-
> 3 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index f5ffca24def4..6c0e0c614b94 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2752,7 +2752,7 @@ static const struct file_operations amdgpu_driver_kms_fops = {
> .compat_ioctl = amdgpu_kms_compat_ioctl,
> #endif
> #ifdef CONFIG_PROC_FS
> - .show_fdinfo = amdgpu_show_fdinfo
> + .show_fdinfo = drm_show_fdinfo,
> #endif
> };
>
> @@ -2807,6 +2807,7 @@ static const struct drm_driver amdgpu_kms_driver = {
> .dumb_map_offset = amdgpu_mode_dumb_mmap,
> .fops = &amdgpu_driver_kms_fops,
> .release = &amdgpu_driver_release_kms,
> + .show_fdinfo = amdgpu_show_fdinfo,
>
> .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> index 99a7855ab1bc..c2fdd5e448d1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> @@ -53,9 +53,8 @@ static const char *amdgpu_ip_name[AMDGPU_HW_IP_NUM] = {
> [AMDGPU_HW_IP_VCN_JPEG] = "jpeg",
> };
>
> -void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
> +void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
> {
> - struct drm_file *file = f->private_data;
> struct amdgpu_device *adev = drm_to_adev(file->minor->dev);
> struct amdgpu_fpriv *fpriv = file->driver_priv;
> struct amdgpu_vm *vm = &fpriv->vm;
> @@ -86,18 +85,15 @@ void amdgpu_show_fdinfo(struct seq_file *m, struct file *f)
> * ******************************************************************
> */
>
> - seq_printf(m, "pasid:\t%u\n", fpriv->vm.pasid);
> - seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name);
> - seq_printf(m, "drm-pdev:\t%04x:%02x:%02x.%d\n", domain, bus, dev, fn);
> - seq_printf(m, "drm-client-id:\t%Lu\n", vm->immediate.fence_context);
> - seq_printf(m, "drm-memory-vram:\t%llu KiB\n", vram_mem/1024UL);
> - seq_printf(m, "drm-memory-gtt: \t%llu KiB\n", gtt_mem/1024UL);
> - seq_printf(m, "drm-memory-cpu: \t%llu KiB\n", cpu_mem/1024UL);
> + drm_printf(p, "pasid:\t%u\n", fpriv->vm.pasid);
> + drm_printf(p, "drm-memory-vram:\t%llu KiB\n", vram_mem/1024UL);
> + drm_printf(p, "drm-memory-gtt: \t%llu KiB\n", gtt_mem/1024UL);
> + drm_printf(p, "drm-memory-cpu: \t%llu KiB\n", cpu_mem/1024UL);
> for (hw_ip = 0; hw_ip < AMDGPU_HW_IP_NUM; ++hw_ip) {
> if (!usage[hw_ip])
> continue;
>
> - seq_printf(m, "drm-engine-%s:\t%Ld ns\n", amdgpu_ip_name[hw_ip],
> + drm_printf(p, "drm-engine-%s:\t%Ld ns\n", amdgpu_ip_name[hw_ip],
> ktime_to_ns(usage[hw_ip]));
> }
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h
> index e86834bfea1d..0398f5a159ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h
> @@ -37,6 +37,6 @@
> #include "amdgpu_ids.h"
>
> uint32_t amdgpu_get_ip_count(struct amdgpu_device *adev, int id);
> -void amdgpu_show_fdinfo(struct seq_file *m, struct file *f);
> +void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file);
>
> #endif
On 12/04/2023 23:42, Rob Clark wrote:
> From: Rob Clark <[email protected]>
There is more do to here to remove my client->id fully (would now be
dead code) so maybe easiest if you drop this patch and I do it after you
land this and it propagates to our branches? I'd like to avoid pain with
conflicts if possible..
Regards,
Tvrtko
>
> Signed-off-by: Rob Clark <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_driver.c | 3 ++-
> drivers/gpu/drm/i915/i915_drm_client.c | 18 +++++-------------
> drivers/gpu/drm/i915/i915_drm_client.h | 2 +-
> 3 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index db7a86def7e2..0d91f85f8b97 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1696,7 +1696,7 @@ static const struct file_operations i915_driver_fops = {
> .compat_ioctl = i915_ioc32_compat_ioctl,
> .llseek = noop_llseek,
> #ifdef CONFIG_PROC_FS
> - .show_fdinfo = i915_drm_client_fdinfo,
> + .show_fdinfo = drm_show_fdinfo,
> #endif
> };
>
> @@ -1796,6 +1796,7 @@ static const struct drm_driver i915_drm_driver = {
> .open = i915_driver_open,
> .lastclose = i915_driver_lastclose,
> .postclose = i915_driver_postclose,
> + .show_fdinfo = i915_drm_client_fdinfo,
>
> .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
> index b09d1d386574..4a77e5e47f79 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.c
> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
> @@ -101,7 +101,7 @@ static u64 busy_add(struct i915_gem_context *ctx, unsigned int class)
> }
>
> static void
> -show_client_class(struct seq_file *m,
> +show_client_class(struct drm_printer *p,
> struct i915_drm_client *client,
> unsigned int class)
> {
> @@ -117,22 +117,20 @@ show_client_class(struct seq_file *m,
> rcu_read_unlock();
>
> if (capacity)
> - seq_printf(m, "drm-engine-%s:\t%llu ns\n",
> + drm_printf(p, "drm-engine-%s:\t%llu ns\n",
> uabi_class_names[class], total);
>
> if (capacity > 1)
> - seq_printf(m, "drm-engine-capacity-%s:\t%u\n",
> + drm_printf(p, "drm-engine-capacity-%s:\t%u\n",
> uabi_class_names[class],
> capacity);
> }
>
> -void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
> +void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file)
> {
> - struct drm_file *file = f->private_data;
> struct drm_i915_file_private *file_priv = file->driver_priv;
> struct drm_i915_private *i915 = file_priv->dev_priv;
> struct i915_drm_client *client = file_priv->client;
> - struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> unsigned int i;
>
> /*
> @@ -141,12 +139,6 @@ void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
> * ******************************************************************
> */
>
> - seq_printf(m, "drm-driver:\t%s\n", i915->drm.driver->name);
> - seq_printf(m, "drm-pdev:\t%04x:%02x:%02x.%d\n",
> - pci_domain_nr(pdev->bus), pdev->bus->number,
> - PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> - seq_printf(m, "drm-client-id:\t%u\n", client->id);
> -
> /*
> * Temporarily skip showing client engine information with GuC submission till
> * fetching engine busyness is implemented in the GuC submission backend
> @@ -155,6 +147,6 @@ void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
> return;
>
> for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++)
> - show_client_class(m, client, i);
> + show_client_class(p, client, i);
> }
> #endif
> diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
> index 69496af996d9..ef85fef45de5 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.h
> +++ b/drivers/gpu/drm/i915/i915_drm_client.h
> @@ -60,7 +60,7 @@ static inline void i915_drm_client_put(struct i915_drm_client *client)
> struct i915_drm_client *i915_drm_client_add(struct i915_drm_clients *clients);
>
> #ifdef CONFIG_PROC_FS
> -void i915_drm_client_fdinfo(struct seq_file *m, struct file *f);
> +void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
> #endif
>
> void i915_drm_clients_fini(struct i915_drm_clients *clients);
On Thu, Apr 13, 2023 at 6:07 AM Tvrtko Ursulin
<[email protected]> wrote:
>
>
> On 12/04/2023 23:42, Rob Clark wrote:
> > From: Rob Clark <[email protected]>
>
> There is more do to here to remove my client->id fully (would now be
> dead code) so maybe easiest if you drop this patch and I do it after you
> land this and it propagates to our branches? I'd like to avoid pain with
> conflicts if possible..
That is fine by me
BR,
-R
> Regards,
>
> Tvrtko
>
> >
> > Signed-off-by: Rob Clark <[email protected]>
> > ---
> > drivers/gpu/drm/i915/i915_driver.c | 3 ++-
> > drivers/gpu/drm/i915/i915_drm_client.c | 18 +++++-------------
> > drivers/gpu/drm/i915/i915_drm_client.h | 2 +-
> > 3 files changed, 8 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > index db7a86def7e2..0d91f85f8b97 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -1696,7 +1696,7 @@ static const struct file_operations i915_driver_fops = {
> > .compat_ioctl = i915_ioc32_compat_ioctl,
> > .llseek = noop_llseek,
> > #ifdef CONFIG_PROC_FS
> > - .show_fdinfo = i915_drm_client_fdinfo,
> > + .show_fdinfo = drm_show_fdinfo,
> > #endif
> > };
> >
> > @@ -1796,6 +1796,7 @@ static const struct drm_driver i915_drm_driver = {
> > .open = i915_driver_open,
> > .lastclose = i915_driver_lastclose,
> > .postclose = i915_driver_postclose,
> > + .show_fdinfo = i915_drm_client_fdinfo,
> >
> > .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
> > index b09d1d386574..4a77e5e47f79 100644
> > --- a/drivers/gpu/drm/i915/i915_drm_client.c
> > +++ b/drivers/gpu/drm/i915/i915_drm_client.c
> > @@ -101,7 +101,7 @@ static u64 busy_add(struct i915_gem_context *ctx, unsigned int class)
> > }
> >
> > static void
> > -show_client_class(struct seq_file *m,
> > +show_client_class(struct drm_printer *p,
> > struct i915_drm_client *client,
> > unsigned int class)
> > {
> > @@ -117,22 +117,20 @@ show_client_class(struct seq_file *m,
> > rcu_read_unlock();
> >
> > if (capacity)
> > - seq_printf(m, "drm-engine-%s:\t%llu ns\n",
> > + drm_printf(p, "drm-engine-%s:\t%llu ns\n",
> > uabi_class_names[class], total);
> >
> > if (capacity > 1)
> > - seq_printf(m, "drm-engine-capacity-%s:\t%u\n",
> > + drm_printf(p, "drm-engine-capacity-%s:\t%u\n",
> > uabi_class_names[class],
> > capacity);
> > }
> >
> > -void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
> > +void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file)
> > {
> > - struct drm_file *file = f->private_data;
> > struct drm_i915_file_private *file_priv = file->driver_priv;
> > struct drm_i915_private *i915 = file_priv->dev_priv;
> > struct i915_drm_client *client = file_priv->client;
> > - struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
> > unsigned int i;
> >
> > /*
> > @@ -141,12 +139,6 @@ void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
> > * ******************************************************************
> > */
> >
> > - seq_printf(m, "drm-driver:\t%s\n", i915->drm.driver->name);
> > - seq_printf(m, "drm-pdev:\t%04x:%02x:%02x.%d\n",
> > - pci_domain_nr(pdev->bus), pdev->bus->number,
> > - PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> > - seq_printf(m, "drm-client-id:\t%u\n", client->id);
> > -
> > /*
> > * Temporarily skip showing client engine information with GuC submission till
> > * fetching engine busyness is implemented in the GuC submission backend
> > @@ -155,6 +147,6 @@ void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
> > return;
> >
> > for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++)
> > - show_client_class(m, client, i);
> > + show_client_class(p, client, i);
> > }
> > #endif
> > diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
> > index 69496af996d9..ef85fef45de5 100644
> > --- a/drivers/gpu/drm/i915/i915_drm_client.h
> > +++ b/drivers/gpu/drm/i915/i915_drm_client.h
> > @@ -60,7 +60,7 @@ static inline void i915_drm_client_put(struct i915_drm_client *client)
> > struct i915_drm_client *i915_drm_client_add(struct i915_drm_clients *clients);
> >
> > #ifdef CONFIG_PROC_FS
> > -void i915_drm_client_fdinfo(struct seq_file *m, struct file *f);
> > +void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
> > #endif
> >
> > void i915_drm_clients_fini(struct i915_drm_clients *clients);
On Wed, 12 Apr 2023 at 23:43, Rob Clark <[email protected]> wrote:
> +/**
> + * enum drm_gem_object_status - bitmask of object state for fdinfo reporting
> + * @DRM_GEM_OBJECT_RESIDENT: object is resident in memory (ie. not unpinned)
> + * @DRM_GEM_OBJECT_PURGEABLE: object marked as purgeable by userspace
> + *
> + * Bitmask of status used for fdinfo memory stats, see &drm_gem_object_funcs.status
> + * and drm_show_fdinfo(). Note that an object can DRM_GEM_OBJECT_PURGEABLE if
> + * it still active or not resident, in which case drm_show_fdinfo() will not
nit: s/can/can be/;s/if it still/if it is still/
> + * account for it as purgeable. So drivers do not need to check if the buffer
> + * is idle and resident to return this bit. (Ie. userspace can mark a buffer
> + * as purgeable even while it is still busy on the GPU.. it does not _actually_
> + * become puregeable until it becomes idle. The status gem object func does
nit: s/puregeable/purgeable/
I think we want a similar note in the drm-usage-stats.rst file.
With the above the whole series is:
Reviewed-by: Emil Velikov <[email protected]>
Fwiw: Keeping the i915 patch as part of this series would be great.
Sure i915_drm_client->id becomes dead code, but it's a piece one can
live with for a release or two. Then again it's not my call to make.
HTH
Emil
On 21/04/2023 11:26, Emil Velikov wrote:
> On Wed, 12 Apr 2023 at 23:43, Rob Clark <[email protected]> wrote:
>
>> +/**
>> + * enum drm_gem_object_status - bitmask of object state for fdinfo reporting
>> + * @DRM_GEM_OBJECT_RESIDENT: object is resident in memory (ie. not unpinned)
>> + * @DRM_GEM_OBJECT_PURGEABLE: object marked as purgeable by userspace
>> + *
>> + * Bitmask of status used for fdinfo memory stats, see &drm_gem_object_funcs.status
>> + * and drm_show_fdinfo(). Note that an object can DRM_GEM_OBJECT_PURGEABLE if
>> + * it still active or not resident, in which case drm_show_fdinfo() will not
>
> nit: s/can/can be/;s/if it still/if it is still/
>
>> + * account for it as purgeable. So drivers do not need to check if the buffer
>> + * is idle and resident to return this bit. (Ie. userspace can mark a buffer
>> + * as purgeable even while it is still busy on the GPU.. it does not _actually_
>> + * become puregeable until it becomes idle. The status gem object func does
>
> nit: s/puregeable/purgeable/
>
>
> I think we want a similar note in the drm-usage-stats.rst file.
>
> With the above the whole series is:
> Reviewed-by: Emil Velikov <[email protected]>
Have you maybe noticed my slightly alternative proposal? (*) I am not a
fan of putting this flavour of accounting into the core with no way to
opt out. I think it leaves no option but to add more keys in the future
for any driver which will not be happy with the core accounting.
*) https://patchwork.freedesktop.org/series/116581/
> Fwiw: Keeping the i915 patch as part of this series would be great.
> Sure i915_drm_client->id becomes dead code, but it's a piece one can
> live with for a release or two. Then again it's not my call to make.
Rob can take the i915 patch from my RFC too.
Regards,
Tvrtko
On Fri, 21 Apr 2023 at 12:23, Tvrtko Ursulin
<[email protected]> wrote:
> On 21/04/2023 11:26, Emil Velikov wrote:
> > On Wed, 12 Apr 2023 at 23:43, Rob Clark <[email protected]> wrote:
> >
> >> +/**
> >> + * enum drm_gem_object_status - bitmask of object state for fdinfo reporting
> >> + * @DRM_GEM_OBJECT_RESIDENT: object is resident in memory (ie. not unpinned)
> >> + * @DRM_GEM_OBJECT_PURGEABLE: object marked as purgeable by userspace
> >> + *
> >> + * Bitmask of status used for fdinfo memory stats, see &drm_gem_object_funcs.status
> >> + * and drm_show_fdinfo(). Note that an object can DRM_GEM_OBJECT_PURGEABLE if
> >> + * it still active or not resident, in which case drm_show_fdinfo() will not
> >
> > nit: s/can/can be/;s/if it still/if it is still/
> >
> >> + * account for it as purgeable. So drivers do not need to check if the buffer
> >> + * is idle and resident to return this bit. (Ie. userspace can mark a buffer
> >> + * as purgeable even while it is still busy on the GPU.. it does not _actually_
> >> + * become puregeable until it becomes idle. The status gem object func does
> >
> > nit: s/puregeable/purgeable/
> >
> >
> > I think we want a similar note in the drm-usage-stats.rst file.
> >
> > With the above the whole series is:
> > Reviewed-by: Emil Velikov <[email protected]>
>
> Have you maybe noticed my slightly alternative proposal? (*) I am not a
> fan of putting this flavour of accounting into the core with no way to
> opt out. I think it leaves no option but to add more keys in the future
> for any driver which will not be happy with the core accounting.
>
> *) https://patchwork.freedesktop.org/series/116581/
>
Indeed I saw it. Not a fan of it, I'm afraid.
> > Fwiw: Keeping the i915 patch as part of this series would be great.
> > Sure i915_drm_client->id becomes dead code, but it's a piece one can
> > live with for a release or two. Then again it's not my call to make.
>
> Rob can take the i915 patch from my RFC too.
>
Indeed.
-Emil
On 21/04/2023 12:45, Emil Velikov wrote:
> On Fri, 21 Apr 2023 at 12:23, Tvrtko Ursulin
> <[email protected]> wrote:
>
>> On 21/04/2023 11:26, Emil Velikov wrote:
>>> On Wed, 12 Apr 2023 at 23:43, Rob Clark <[email protected]> wrote:
>>>
>>>> +/**
>>>> + * enum drm_gem_object_status - bitmask of object state for fdinfo reporting
>>>> + * @DRM_GEM_OBJECT_RESIDENT: object is resident in memory (ie. not unpinned)
>>>> + * @DRM_GEM_OBJECT_PURGEABLE: object marked as purgeable by userspace
>>>> + *
>>>> + * Bitmask of status used for fdinfo memory stats, see &drm_gem_object_funcs.status
>>>> + * and drm_show_fdinfo(). Note that an object can DRM_GEM_OBJECT_PURGEABLE if
>>>> + * it still active or not resident, in which case drm_show_fdinfo() will not
>>>
>>> nit: s/can/can be/;s/if it still/if it is still/
>>>
>>>> + * account for it as purgeable. So drivers do not need to check if the buffer
>>>> + * is idle and resident to return this bit. (Ie. userspace can mark a buffer
>>>> + * as purgeable even while it is still busy on the GPU.. it does not _actually_
>>>> + * become puregeable until it becomes idle. The status gem object func does
>>>
>>> nit: s/puregeable/purgeable/
>>>
>>>
>>> I think we want a similar note in the drm-usage-stats.rst file.
>>>
>>> With the above the whole series is:
>>> Reviewed-by: Emil Velikov <[email protected]>
>>
>> Have you maybe noticed my slightly alternative proposal? (*) I am not a
>> fan of putting this flavour of accounting into the core with no way to
>> opt out. I think it leaves no option but to add more keys in the future
>> for any driver which will not be happy with the core accounting.
>>
>> *) https://patchwork.freedesktop.org/series/116581/
>>
>
> Indeed I saw it. Not a fan of it, I'm afraid.
Hard to guess the reasons. :)
Anyway, at a minimum I suggest that if the no opt out version has to go
in, it is clearly documented drm-*-memory-* is *not* about the full
memory use of the client, but about memory belonging to user visible
buffer objects *only*. Possibly going as far as naming the keys as
drm-user-bo-memory-... That way there is a way to implement proper
drm-*-memory- in the future.
Regards,
Tvrtko
>>> Fwiw: Keeping the i915 patch as part of this series would be great.
>>> Sure i915_drm_client->id becomes dead code, but it's a piece one can
>>> live with for a release or two. Then again it's not my call to make.
>>
>> Rob can take the i915 patch from my RFC too.
>>
>
> Indeed.
>
> -Emil
On Fri, Apr 21, 2023 at 4:59 AM Tvrtko Ursulin
<[email protected]> wrote:
>
>
> On 21/04/2023 12:45, Emil Velikov wrote:
> > On Fri, 21 Apr 2023 at 12:23, Tvrtko Ursulin
> > <[email protected]> wrote:
> >
> >> On 21/04/2023 11:26, Emil Velikov wrote:
> >>> On Wed, 12 Apr 2023 at 23:43, Rob Clark <[email protected]> wrote:
> >>>
> >>>> +/**
> >>>> + * enum drm_gem_object_status - bitmask of object state for fdinfo reporting
> >>>> + * @DRM_GEM_OBJECT_RESIDENT: object is resident in memory (ie. not unpinned)
> >>>> + * @DRM_GEM_OBJECT_PURGEABLE: object marked as purgeable by userspace
> >>>> + *
> >>>> + * Bitmask of status used for fdinfo memory stats, see &drm_gem_object_funcs.status
> >>>> + * and drm_show_fdinfo(). Note that an object can DRM_GEM_OBJECT_PURGEABLE if
> >>>> + * it still active or not resident, in which case drm_show_fdinfo() will not
> >>>
> >>> nit: s/can/can be/;s/if it still/if it is still/
> >>>
> >>>> + * account for it as purgeable. So drivers do not need to check if the buffer
> >>>> + * is idle and resident to return this bit. (Ie. userspace can mark a buffer
> >>>> + * as purgeable even while it is still busy on the GPU.. it does not _actually_
> >>>> + * become puregeable until it becomes idle. The status gem object func does
> >>>
> >>> nit: s/puregeable/purgeable/
> >>>
> >>>
> >>> I think we want a similar note in the drm-usage-stats.rst file.
> >>>
> >>> With the above the whole series is:
> >>> Reviewed-by: Emil Velikov <[email protected]>
> >>
> >> Have you maybe noticed my slightly alternative proposal? (*) I am not a
> >> fan of putting this flavour of accounting into the core with no way to
> >> opt out. I think it leaves no option but to add more keys in the future
> >> for any driver which will not be happy with the core accounting.
> >>
> >> *) https://patchwork.freedesktop.org/series/116581/
> >>
> >
> > Indeed I saw it. Not a fan of it, I'm afraid.
>
> Hard to guess the reasons. :)
>
> Anyway, at a minimum I suggest that if the no opt out version has to go
> in, it is clearly documented drm-*-memory-* is *not* about the full
> memory use of the client, but about memory belonging to user visible
> buffer objects *only*. Possibly going as far as naming the keys as
> drm-user-bo-memory-... That way there is a way to implement proper
> drm-*-memory- in the future.
I'll go back to the helper approach, just been distracted by a few
other balls in the air.. should hopefully get to it in the next couple
days
BR,
-R
> Regards,
>
> Tvrtko
>
> >>> Fwiw: Keeping the i915 patch as part of this series would be great.
> >>> Sure i915_drm_client->id becomes dead code, but it's a piece one can
> >>> live with for a release or two. Then again it's not my call to make.
> >>
> >> Rob can take the i915 patch from my RFC too.
> >>
> >
> > Indeed.
> >
> > -Emil