Re: [Intel-xe] [RFC 2/4] drm/xe: Add memory directory for vram attributes

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

On 11/18/2023 6:55 AM, Welty, Brian wrote:
>
>
> On 11/17/2023 1:37 PM, Rodrigo Vivi wrote:
>> On Thu, Nov 16, 2023 at 08:00:41PM +0530, Sujaritha Sundaresan wrote:
>>> Adding a new "memory" directory under for vram attributes
>>> under the exisiting tile<n> directory.
>>>
>>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>>
>> Cc: Brian Welty <brian.welty@intel.com>
>>
>> Brian, should this directory be under the tile, or maybe we create
>> separated xe_vram_sysfs components for
>>
>> /sys/..../device/vram<n>
>>
>> and then if it makes sense inside the tile we just have a symbolic
>> link between them?
>
> Oh. Good question. Seems reasonable to me to have vram<n> in both 
> places as you suggest.
>
>>
>> Or better for now go with what Suja suggested and then later adjust
>> with symbolic links if needed?
>
> Yeah, maybe defer doing anything here until later...
>
> But let me ask....
> Was it considered to make the sysfs hierarchy more flat?
>
> Could also go with -
> /sys/..../device/gt<n>
> and:
> /sys/..../device/vram<n>
>
> and remove 'tile' directory?
>
> Could add:
> /sys/..../device/gt<n>/tile
> /sys/..../device/vram<n>/tile
> if feel is important to communicate which tile they are in.
>
> I don't know, just an idea.  Feel free to ignore.
>
> -Brian
>
I think for now I'll just have the /tile/memory and then add a sub 
directory for frequency

under memory for any vram frequency attributes.

Thanks,

Suja

>
>>
>>> ---
>>>   drivers/gpu/drm/xe/xe_tile_sysfs.c | 9 ++++++++-
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_tile_sysfs.c 
>>> b/drivers/gpu/drm/xe/xe_tile_sysfs.c
>>> index 16376607c68f..d61c6fb1df40 100644
>>> --- a/drivers/gpu/drm/xe/xe_tile_sysfs.c
>>> +++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c
>>> @@ -46,6 +46,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,8 +65,14 @@ void xe_tile_sysfs_init(struct xe_tile *tile)
>>>         tile->sysfs = &kt->base;
>>>   +    kobj = kobject_create_and_add("memory", tile->sysfs);
>>> +    if (!kobj) {
>>> +        drm_warn(&xe->drm, "%s failed, err: %d\n", __func__, -ENOMEM);
>>> +        return;
>>> +    }
>>> +
>>>       if (IS_DGFX(xe) && xe->info.platform != XE_DG1 &&
>>> -        sysfs_create_file(tile->sysfs, physical_memsize_attr))
>>> +        sysfs_create_file(kobj, physical_memsize_attr))
>>>           drm_warn(&xe->drm,
>>>                "Sysfs creation to read addr_range per tile failed\n");
>>>   --
>>> 2.25.1
>>>