[PATCH v2 1/3] drm/xe: Move xe_device_sysfs_init() to xe_pci_probe()

[Riana Tauro]

Hi Raag

On 4/16/2025 4:19 PM, Raag Jadav wrote:
> On Wed, Apr 16, 2025 at 03:34:43PM +0530, Riana Tauro wrote:
>> On 4/16/2025 1:25 PM, Riana Tauro wrote:
>>> Hi Raag
>>>
>>> On 4/3/2025 11:17 PM, Raag Jadav wrote:
>>>> Move xe_device_sysfs_init() to xe_pci_probe() so that we can expose
>>>> non-PM attributes as well.
>>>>
>>>> Signed-off-by: Raag Jadav <raag.jadav at intel.com>
>>>> ---
>>>>    drivers/gpu/drm/xe/xe_device_sysfs.c | 11 +++++++----
>>>>    drivers/gpu/drm/xe/xe_pci.c          |  6 ++++++
>>>>    drivers/gpu/drm/xe/xe_pm.c           |  5 -----
>>>>    3 files changed, 13 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/
>>>> xe/xe_device_sysfs.c
>>>> index 7efbd4c52791..d4c73acea1cf 100644
>>>> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
>>>> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
>>>> @@ -67,7 +67,8 @@ static void xe_device_sysfs_fini(void *arg)
>>>>    {
>>>>        struct xe_device *xe = arg;
>>>> -    sysfs_remove_file(&xe->drm.dev->kobj,
>>>> &dev_attr_vram_d3cold_threshold.attr);
>>>> +    if (xe->d3cold.capable)
>>>> +        sysfs_remove_file(&xe->drm.dev->kobj,
>>>> &dev_attr_vram_d3cold_threshold.attr);
>>>>    }
>>>>    int xe_device_sysfs_init(struct xe_device *xe)
>>>> @@ -75,9 +76,11 @@ int xe_device_sysfs_init(struct xe_device *xe)
>>>>        struct device *dev = xe->drm.dev;
>>>>        int ret;
>>>> -    ret = sysfs_create_file(&dev->kobj,
>>>> &dev_attr_vram_d3cold_threshold.attr);
>>>> -    if (ret)
>>>> -        return ret;
>>>> +    if (xe->d3cold.capable) {
>>>> +        ret = sysfs_create_file(&dev->kobj,
>>>> &dev_attr_vram_d3cold_threshold.attr);
>>>> +        if (ret)
>>>> +            return ret;
>>>> +    }
>>>>        return devm_add_action_or_reset(dev, xe_device_sysfs_fini, xe);
>>>>    }
>>>> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>>>> index 58347fa91841..246682b2a2be 100644
>>>> --- a/drivers/gpu/drm/xe/xe_pci.c
>>>> +++ b/drivers/gpu/drm/xe/xe_pci.c
>>>> @@ -18,6 +18,7 @@
>>>>    #include "display/xe_display.h"
>>>>    #include "regs/xe_gt_regs.h"
>>>>    #include "xe_device.h"
>>>> +#include "xe_device_sysfs.h"
>>>>    #include "xe_drv.h"
>>>>    #include "xe_gt.h"
>>>>    #include "xe_gt_sriov_vf.h"
>>>> @@ -864,6 +865,11 @@ static int xe_pci_probe(struct pci_dev *pdev,
>>>> const struct pci_device_id *ent)
>>>>        if (err)
>>>>            goto err_driver_cleanup;
>>>> +    /* We have PM related attributes, so do it after xe_pm_init(). */
>>>> +    err = xe_device_sysfs_init(xe);
>>> Layering doesn't seem right. This should in xe_device_probe.
>>> d3cold capable can be moved to pm_init_early and this to device_probe
> 
> Are you sure if we can use ttm_manager_type() in pm_init_early()?

you need only d3cold.capable to setup sysfs. This call is using only pci 
calls. So this should work

xe->d3cold.capable = xe_pm_pci_d3cold_capable(xe);

Thanks
Riana>
>> +    if (err)
>>
>> Also this should be a goto
>>
>> if (err)
>>   	goto err_driver_cleanup;
> 
> Sure.
> 
> Raag