Re: [v3 1/2] drm/xe: Add a new memory directory under tile

["Sundaresan, Sujaritha" <sujaritha.sundaresan@intel.com>]

On 12/8/2023 10:33 AM, Upadhyay, Tejas wrote:
>
>> -----Original Message-----
>> From: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
>> Sent: Thursday, December 7, 2023 8:48 PM
>> To: intel-xe@lists.freedesktop.org
>> Cc: Upadhyay, Tejas <tejas.upadhyay@intel.com>; Tauro, Riana
>> <riana.tauro@intel.com>; Gupta, Anshuman <anshuman.gupta@intel.com>;
>> Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>
>> Subject: [v3 1/2] drm/xe: Add a new memory directory under tile
>>
>> Add a new memory directory under /device/tile<n> and move
>> physical_vram_size attribute to the new directory.
>>
>> New hierarchy:
>>
>> /device/tile<n>/memory/physical_vram_size_bytes
>>
>> v2: Fix heading typo (Riana)
>>      Fix cleanup error on unload/reload cycle
>>
>> v3: Fix reload error with kobject_put in place
>>      of kobject_del (Tejas)
>>
>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_tile_sysfs.c | 19 ++++++++++++++-----
>>   1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_tile_sysfs.c
>> b/drivers/gpu/drm/xe/xe_tile_sysfs.c
>> index 16376607c68f..64be1f3a38a9 100644
>> --- a/drivers/gpu/drm/xe/xe_tile_sysfs.c
>> +++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c
>> @@ -24,7 +24,8 @@ 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);
>> +	struct kobject *kobj = &kdev->kobj;
>> +	struct xe_tile *tile = kobj_to_tile(kobj->parent);
> As Christmas is approaching, you might want to adjust this in Christmas tree order.
>
>>   	return sysfs_emit(buf, "%llu\n", tile->mem.vram.actual_physical_size);
>>   }
>> @@ -36,8 +37,10 @@ static const struct attribute *physical_memsize_attr =
>>
>>   static void tile_sysfs_fini(struct drm_device *drm, void *arg)  {
>> -	struct xe_tile *tile = arg;
>> +	struct kobject *kobj = arg;
>> +	struct xe_tile *tile = kobj_to_tile(kobj->parent);
> Ditto
>
>> +	kobject_put(kobj);
>>   	kobject_put(tile->sysfs);
>>   }
>>
>> @@ -46,6 +49,7 @@ void xe_tile_sysfs_init(struct xe_tile *tile)
>>   	struct xe_device *xe = tile_to_xe(tile);
>>   	struct device *dev = xe->drm.dev;
>>   	struct kobj_tile *kt;
>> +	struct kobject *kobj;
>>   	int err;
>>
>>   	kt = kzalloc(sizeof(*kt), GFP_KERNEL); @@ -64,12 +68,17 @@ 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))
>> +	kobj = kobject_create_and_add("memory", tile->sysfs);
>> +	if (!kobj) {
>> +		drm_warn(&xe->drm, "%s failed, err: %d\n", __func__, -
>> ENOMEM);
>> +	}
> { } is not required here.
>
> Tejas

Will fix all of the above.

Thanks,

Suja

>> +
>> +	if (kobj && IS_DGFX(xe) && xe->info.platform != XE_DG1 &&
>> +	    sysfs_create_file(kobj, 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);
>> +	err = drmm_add_action_or_reset(&xe->drm, tile_sysfs_fini, kobj);
>>   	if (err) {
>>   		drm_warn(&xe->drm, "%s: drmm_add_action_or_reset
>> failed, err: %d\n",
>>   			 __func__, err);
>> --
>> 2.25.1