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

[Upadhyay, Tejas]

> -----Original Message-----
> From: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>
> Sent: Tuesday, June 6, 2023 5:07 PM
> To: Upadhyay, Tejas <tejas.upadhyay at intel.com>; intel-
> xe at lists.freedesktop.org
> Subject: RE: [Intel-xe] [PATCH 3/3] drm/xe: Add sysfs entry to report per tile
> memory size
> 
> 
> 
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of
> > Tejas Upadhyay
> > Sent: 06 June 2023 16:43
> > To: intel-xe at lists.freedesktop.org
> > Subject: [Intel-xe] [PATCH 3/3] drm/xe: Add sysfs entry to report per
> > tile memory size
> >
> > Add sysfs entry to read per tile physical memory including stolen memory.
> >
> > Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_device_types.h |  5 +++
> >  drivers/gpu/drm/xe/xe_mmio.c         |  1 +
> >  drivers/gpu/drm/xe/xe_tile_sysfs.c   | 47
> > ++++++++++++++++++++++++++++
> >  3 files changed, 53 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> > b/drivers/gpu/drm/xe/xe_device_types.h
> > index db61da4d6212..7b5443aebd00 100644
> > --- a/drivers/gpu/drm/xe/xe_device_types.h
> > +++ b/drivers/gpu/drm/xe/xe_device_types.h
> > @@ -107,6 +107,11 @@ struct xe_tile {
> >
> >  	/** @mem: memory management info for tile */
> >  	struct {
> > +		/**
> > +		 * @actual_physical_mem: Track actual LMEM size
> > +		 * including stolen mem for tile
> > +		 */
> > +		resource_size_t actual_physical_mem;
> >  		/**
> >  		 * @vram: VRAM info for tile.
> >  		 *
> > diff --git a/drivers/gpu/drm/xe/xe_mmio.c
> > b/drivers/gpu/drm/xe/xe_mmio.c index 475b14fe4356..86a7c65c301f
> 100644
> > --- a/drivers/gpu/drm/xe/xe_mmio.c
> > +++ b/drivers/gpu/drm/xe/xe_mmio.c
> > @@ -283,6 +283,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 821bd49de277..d48c599e8cc1 100644
> > --- a/drivers/gpu/drm/xe/xe_tile_sysfs.c
> > +++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c
> > @@ -22,6 +22,43 @@ static struct kobj_type xe_tile_sysfs_kobj_type = {
> >  	.sysfs_ops = &kobj_sysfs_ops,
> >  };
> >
> > +typedef ssize_t (*show)(struct device *dev, struct device_attribute
> > +*attr, char *buf);
> > +
> > +struct xe_ext_attr {
> > +	struct device_attribute attr;
> > +	show xe_show;
> > +};
> > +
> > +	static ssize_t
> > +xe_sysfs_show(struct device *dev, struct device_attribute *attr, char
> > +*buf) {
> > +	ssize_t value;
> > +	struct xe_ext_attr *ea = container_of(attr, struct xe_ext_attr,
> > +attr);
> > +
> > +	value = ea->xe_show(dev, attr, buf);
> > +
> > +	return value;
> > +}
> > +
> > +	static ssize_t
> > +addr_range_show(struct device *kdev, struct device_attribute *attr,
> > +char *buf) {
> > +	struct xe_tile *tile = kobj_to_tile(&kdev->kobj);
> > +
> > +	return sysfs_emit(buf, "%pa\n", &tile->mem.actual_physical_mem);
> > }
> > +
> > +#define XE_DEVICE_ATTR_RO(_name, _show) \
> > +	struct xe_ext_attr dev_attr_##_name = \ { __ATTR(_name, 0444,
> > +xe_sysfs_show, NULL), _show}
> > +
> > +static XE_DEVICE_ATTR_RO(addr_range, addr_range_show);
> > +
> > +static const struct attribute *addr_range_attrs[] = {
> > +	&dev_attr_addr_range.attr.attr,
> > +	NULL
> > +};
> > +
> >  static void tile_sysfs_fini(struct drm_device *drm, void *arg)  {
> >  	struct xe_tile *tile = arg;
> > @@ -50,9 +87,19 @@ int xe_tile_sysfs_init(struct xe_tile *tile)
> >
> >  	tile->sysfs = &kg->base;
> >
> > +	if (sysfs_create_files(tile->sysfs, addr_range_attrs)) {
> > +		drm_err(&tile_to_xe(tile)->drm,
> > +			"Setting up sysfs to read total physical memory per
> > tile failed\n");
> > +		goto err_object;
> Will do kobject_put and return 0. Which is wrong.
> Better to do kobject_put and return err here.

I don't think I have any action to do based on error, clean up was already done. Am I missing something here?

Thanks,
Tejas
> > +	}
> > +
> >  	err = drmm_add_action_or_reset(&tile_to_xe(tile)->drm,
> > tile_sysfs_fini, tile);
> >  	if (err)
> >  		return err;
> >
> > +	if (0) {
> > +err_object:
> > +		kobject_put(&kg->base);
> > +	}
> >  	return 0;
> >  }
> > --
> > 2.25.1