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

[Iddamsetty, Aravind]

On 07-06-2023 15:41, Tejas Upadhyay wrote:
> Add sysfs entry to read per tile physical memory
> including stolen memory.
> 
> V2:
>   - Use DEVICE_ATTR_RO - Aravind
>   - Dont put kobj on sysfs_file_create fail - Himal
>   - Skip addr_range sysfs create for non dgfx - Himal
> 
> 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   | 29 ++++++++++++++++++++++------
>  3 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index db61da4d6212..b8a07826c65f 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -107,6 +107,12 @@ 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 f7a7f996b37f..6febd11d7dd9 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 09a4a8342aec..b5768c735cba 100644
> --- a/drivers/gpu/drm/xe/xe_tile_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c
> @@ -22,6 +22,21 @@ static struct kobj_type xe_tile_sysfs_kobj_type = {
>  	.sysfs_ops = &kobj_sysfs_ops,
>  };
>  
> +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);
> +}
> +
> +static DEVICE_ATTR_RO(addr_range);
> +
> +static const struct attribute *addr_range_attrs[] = {
> +	&dev_attr_addr_range.attr,
> +	NULL
> +};
> +
>  static void tile_sysfs_fini(struct drm_device *drm, void *arg)
>  {
>  	struct xe_tile *tile = arg;
> @@ -37,10 +52,8 @@ int xe_tile_sysfs_init(struct xe_tile *tile)
>  	int err;
>  
>  	kg = kzalloc(sizeof(*kg), GFP_KERNEL);
> -	if (!kg) {
> -		drm_warn(&xe->drm, "%s failed, err: %d\n", __func__, -ENOMEM);
> +	if (!kg)
>  		return -ENOMEM;

your first patch introduced it and you are fixing here should it be
fixed in the first patch itself and why this change as the return from
tile_sysfs_init is not being checked.

> -	}
>  
>  	kobject_init(&kg->base, &xe_tile_sysfs_kobj_type);
>  	kg->tile = tile;
> @@ -48,16 +61,20 @@ int xe_tile_sysfs_init(struct xe_tile *tile)
>  	err = kobject_add(&kg->base, &dev->kobj, "tile%d", tile->id);
>  	if (err) {
>  		kobject_put(&kg->base);
> -		drm_warn(&xe->drm, "failed to register TILE sysfs directory, err: %d\n", err);
> +		drm_err(&xe->drm, "failed to register TILE sysfs directory, err: %d\n", err);

same here, but warn is more appropriate than err.

>  		return err;
>  	}
>  
>  	tile->sysfs = &kg->base;
>  
> +	if (sysfs_create_files(tile->sysfs, addr_range_attrs) && IS_DGFX(xe))

this check is not correct if you want to restrict to DGFX, the check
should be preceeding the sysfs_create_files,
if ( IS_DGFX(xe) && sysfs_create_files())

also you could use sysfs_create_file as you have just one entry.

thanks,
Aravind.
> +		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",
> -			 __func__, err);
> +		drm_err(&xe->drm, "%s: drmm_add_action_or_reset failed, err: %d\n",
> +			__func__, err);
>  		return err;
>  	}
>