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

[Riana Tauro]

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
> 
> Thanks
> Riana > +    if (err)

Also this should be a goto

if (err)
  	goto err_driver_cleanup;>> +        return err;
>> +
>>       drm_dbg(&xe->drm, "d3cold: capable=%s\n",
>>           str_yes_no(xe->d3cold.capable));
>> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
>> index 7b6b754ad6eb..6af1de78dae8 100644
>> --- a/drivers/gpu/drm/xe/xe_pm.c
>> +++ b/drivers/gpu/drm/xe/xe_pm.c
>> @@ -16,7 +16,6 @@
>>   #include "xe_bo.h"
>>   #include "xe_bo_evict.h"
>>   #include "xe_device.h"
>> -#include "xe_device_sysfs.h"
>>   #include "xe_ggtt.h"
>>   #include "xe_gt.h"
>>   #include "xe_guc.h"
>> @@ -306,10 +305,6 @@ int xe_pm_init(struct xe_device *xe)
>>       xe->d3cold.capable = xe_pm_pci_d3cold_capable(xe);
>>       if (xe->d3cold.capable) {
>> -        err = xe_device_sysfs_init(xe);
>> -        if (err)
>> -            return err;
>> -
>>           vram_threshold = vram_threshold_value(xe);
>>           err = xe_pm_set_vram_threshold(xe, vram_threshold);
>>           if (err)
>