On Thu, Jun 12, 2025 at 11:02 AM Lucas De Marchi
<lucas.demar...@intel.com> wrote:
>
> On Thu, Jun 12, 2025 at 05:46:52PM +0100, Tvrtko Ursulin wrote:
> >
> >On 12/06/2025 06:40, Lucas De Marchi wrote:
> >>On Wed, Jun 11, 2025 at 03:51:24PM -0700, Juston Li wrote:
> >>>Add TRACE_GPU_MEM tracepoints for tracking global and per-process GPU
> >>>memory usage.
> >>>
> >>>These are required by VSR on Android 12+ for reporting GPU driver memory
> >>>allocations.
> >>>
> >>>v3:
> >>>- Use now configurable CONFIG_TRACE_GPU_MEM instead of adding a
> >>> per-driver Kconfig (Lucas)
> >>>
> >>>v2:
> >>>- Use u64 as preferred by checkpatch (Tvrtko)
> >>>- Fix errors in comments/Kconfig description (Tvrtko)
> >>>- drop redundant "CONFIG" in Kconfig
> >>>
> >>>Signed-off-by: Juston Li <justo...@chromium.org>
> >>>Reviewed-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
> >>>---
> >>>drivers/gpu/drm/xe/xe_bo.c | 47 ++++++++++++++++++++++++++++
> >>>drivers/gpu/drm/xe/xe_device_types.h | 16 ++++++++++
> >>>2 files changed, 63 insertions(+)
> >>>
> >>>diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> >>>index 4e39188a021ab..89a3d23e3b800 100644
> >>>--- a/drivers/gpu/drm/xe/xe_bo.c
> >>>+++ b/drivers/gpu/drm/xe/xe_bo.c
> >>>@@ -19,6 +19,8 @@
> >>>
> >>>#include <kunit/static_stub.h>
> >>>
> >>>+#include <trace/events/gpu_mem.h>
> >>>+
> >>>#include "xe_device.h"
> >>>#include "xe_dma_buf.h"
> >>>#include "xe_drm_client.h"
> >>>@@ -418,6 +420,35 @@ static void xe_ttm_tt_account_subtract(struct
> >>>xe_device *xe, struct ttm_tt *tt)
> >>> xe_shrinker_mod_pages(xe->mem.shrinker, -(long)tt->num_pages, 0);
> >>>}
> >>>
> >>>+#if IS_ENABLED(CONFIG_TRACE_GPU_MEM)
> >>>+static void update_global_total_pages(struct ttm_device *ttm_dev,
> >>>long num_pages)
> >>>+{
> >>>+ struct xe_device *xe = ttm_to_xe_device(ttm_dev);
> >>>+ u64 global_total_pages =
> >>>+ atomic64_add_return(num_pages, &xe->global_total_pages);
> >>>+
> >>>+ trace_gpu_mem_total(0, 0, global_total_pages << PAGE_SHIFT);
> >>>+}
> >>>+
> >>>+static void update_process_mem(struct drm_file *file, ssize_t size)
> >>>+{
> >>>+ struct xe_file *xef = to_xe_file(file);
> >>>+ u64 process_mem = atomic64_add_return(size, &xef->process_mem);
> >>>+
> >>>+ rcu_read_lock(); /* Locks file->pid! */
> >>>+ trace_gpu_mem_total(0, pid_nr(rcu_dereference(file->pid)),
> >>>process_mem);
> >>
> >>Isn't the first arg supposed to be the gpu id? Doesn't this become
> >>invalid when I have e.g. LNL + BMG and the trace is enabled?
> >
> >Good point.
> >
> >u32 gpu_id does not seem possible to map to anything useful.
>
> maybe minor_id? although I'm not sure if the intention is to share this
> outside drm as seems the case.
Yes, that was for render minor in the case of drm.
>
> >
> >Shall we replace it with a string obtained from dev_name(struct device
> >*) ? As only Android parses them I think we can get still away with
> >changing the tracepoints ABI.
>
> works for me too. Is Android actually parsing it or just ignoring?
> Because afaics it's always 0 in msm.
Android has used it as part of the bpf map key:
https://cs.android.com/android/platform/superproject/main/+/main:frameworks/native/services/gpuservice/bpfprogs/gpuMem.c
>
> Lucas De Marchi
