[Intel-xe] [PATCH V4 3/3] drm/xe: Add sysfs entry to report per tile memory size

[Upadhyay, Tejas]

> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi at intel.com>
> Sent: Tuesday, June 27, 2023 11:38 PM
> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>
> Cc: intel-xe at lists.freedesktop.org
> Subject: Re: [Intel-xe] [PATCH V4 3/3] drm/xe: Add sysfs entry to report per
> tile memory size
> 
> On Thu, Jun 22, 2023 at 12:58:18PM +0530, Tejas Upadhyay wrote:
> >Add sysfs entry to read per tile physical memory including stolen
> >memory.
> >
> >V4:
> >  - %s/addr_range/physical_vram_size_byes, make it
> >    user readable name - Joonas/Aravind
> >  - Display in bytes - Joonas/Aravind
> >V3:
> >  - Exclude DG1, replace sysfs_create_file/files - Aravind
> >V2:
> >  - Use DEVICE_ATTR_RO - Aravind
> >  - Dont put kobj on sysfs_file_create fail - Himal
> >  - Skip addr_range sysfs create for non dgfx - Himal
> >
> >Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
> >Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> >---
> > drivers/gpu/drm/xe/xe_device_types.h |  6 ++++++
> > drivers/gpu/drm/xe/xe_mmio.c         |  1 +
> > drivers/gpu/drm/xe/xe_tile_sysfs.c   | 19 +++++++++++++++++++
> > 3 files changed, 26 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> >b/drivers/gpu/drm/xe/xe_device_types.h
> >index f2d4298fb0a1..dada97352330 100644
> >--- a/drivers/gpu/drm/xe/xe_device_types.h
> >+++ b/drivers/gpu/drm/xe/xe_device_types.h
> >@@ -108,6 +108,12 @@ struct xe_tile {
> >
> > 	/** @mem: memory management info for tile */
> > 	struct {
> >+		/**
> >+		 * @actual_physical_mem: Track actual VRAM size
> >+		 * including stolen mem for tile
> >+		 */
> >+		resource_size_t actual_physical_mem;
> 
> should probably be inside the vram struct below? Then you probably need to
> tweak this doc
> 
> 	/** @size: size of VRAM. */
> 	resource_size_t size;
> 
> to explain how these are different... i.e. that while your physical_vram_size
> doesn't exclude anything, "size" excludes reserved portions (e.g. the stolen
> memory).  So...
> 
> vram {
> 	io_size - vram that can be accessed by the CPU through the io
> mapping
> 	size - probably rename to "usable_size"?, the total vram minus
> reserved
> 	       portions
> 	actual_physical_size - ...
> }

I agree, name "size" confused me when I looked at vram code logic first, usable_size give more meaning to it. I will change accordingly, thanks!

Tejas
> 
> Lucas De Marchi
> 
> >+
> > 		/**
> > 		 * @vram: VRAM info for tile.
> > 		 *
> >diff --git a/drivers/gpu/drm/xe/xe_mmio.c
> >b/drivers/gpu/drm/xe/xe_mmio.c index f1336803b915..4bb8e8c3080e
> 100644
> >--- a/drivers/gpu/drm/xe/xe_mmio.c
> >+++ b/drivers/gpu/drm/xe/xe_mmio.c
> >@@ -277,6 +277,7 @@ int xe_mmio_probe_vram(struct xe_device *xe)
> > 		if (err)
> > 			return err;
> >
> >+		tile->mem.actual_physical_mem = tile_size;
> > 		tile->mem.vram.io_start = xe->mem.vram.io_start +
> tile_offset;
> > 		tile->mem.vram.io_size = min_t(u64, vram_size, io_size);
> >
> >diff --git a/drivers/gpu/drm/xe/xe_tile_sysfs.c
> >b/drivers/gpu/drm/xe/xe_tile_sysfs.c
> >index 386c6accd4dd..6b559e2f2f7b 100644
> >--- a/drivers/gpu/drm/xe/xe_tile_sysfs.c
> >+++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c
> >@@ -20,6 +20,20 @@ static struct kobj_type xe_tile_sysfs_kobj_type = {
> > 	.sysfs_ops = &kobj_sysfs_ops,
> > };
> >
> >+static ssize_t
> >+physical_vram_size_bytes_show(struct device *kdev, struct
> device_attribute *attr,
> >+			      char *buf)
> >+{
> >+	struct xe_tile *tile = kobj_to_tile(&kdev->kobj);
> >+
> >+	return sysfs_emit(buf, "%llu\n", tile->mem.actual_physical_mem); }
> >+
> >+static DEVICE_ATTR_RO(physical_vram_size_bytes);
> >+
> >+static const struct attribute *physical_memsize_attr =
> >+	&dev_attr_physical_vram_size_bytes.attr;
> >+
> > static void tile_sysfs_fini(struct drm_device *drm, void *arg)  {
> > 	struct xe_tile *tile = arg;
> >@@ -50,6 +64,11 @@ void xe_tile_sysfs_init(struct xe_tile *tile)
> >
> > 	tile->sysfs = &kt->base;
> >
> >+	if (IS_DGFX(xe) && xe->info.platform != XE_DG1 &&
> >+	    sysfs_create_file(tile->sysfs, physical_memsize_attr))
> >+		drm_warn(&xe->drm,
> >+			 "Sysfs creation to read addr_range per tile failed\n");
> >+
> > 	err = drmm_add_action_or_reset(&xe->drm, tile_sysfs_fini, tile);
> > 	if (err) {
> > 		drm_warn(&xe->drm, "%s: drmm_add_action_or_reset
> failed, err: %d\n",
> >--
> >2.25.1
> >