On 3/5/19 1:26 PM, Koenig, Christian wrote: > Am 05.03.19 um 19:24 schrieb Grodzovsky, Andrey: >> On 3/5/19 1:18 PM, Koenig, Christian wrote: >>> Am 05.03.19 um 18:47 schrieb Andrey Grodzovsky: >>>> For each device a file xgmi_device_id is created. >>>> On the first device a subdirectory named xgmi_hive_info is created, >>>> It contains a file named hive_id and symlinks named node 1-4 linking >>>> to each device in the hive. >>>> >>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 145 ++++++++++++++++++++++++++++++- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h | 6 +- >>>> 2 files changed, 146 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c >>>> index 407dd16c..394be10 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c >>>> @@ -34,12 +34,132 @@ static DEFINE_MUTEX(xgmi_mutex); >>>> static struct amdgpu_hive_info xgmi_hives[AMDGPU_MAX_XGMI_HIVE]; >>>> static unsigned hive_count = 0; >>>> >>>> - >>>> void *amdgpu_xgmi_hive_try_lock(struct amdgpu_hive_info *hive) >>>> { >>>> return &hive->device_list; >>>> } >>>> >>>> +static ssize_t amdgpu_xgmi_show_hive_id(struct device *dev, >>>> + struct device_attribute *attr, char *buf) >>>> +{ >>>> + struct amdgpu_hive_info *hive = >>>> + container_of(attr, struct amdgpu_hive_info, dev_attr); >>>> + >>>> + return snprintf(buf, PAGE_SIZE, "%llu\n", hive->hive_id); >>>> +} >>>> + >>>> +static int amdgpu_xgmi_sysfs_create(struct amdgpu_device *adev, >>>> + struct amdgpu_hive_info *hive) >>>> +{ >>>> + int ret = 0; >>>> + >>>> + if (WARN_ON(hive->kobj)) >>>> + return -1; >>> Please return proper error codes here. >>> >>>> + >>>> + hive->kobj = kobject_create_and_add("xgmi_hive_info", &adev->dev->kobj); >>>> + if (!hive->kobj) { >>>> + dev_err(adev->dev, "XGMI: Failed to allocate sysfs entry!\n"); >>>> + return -1; >>>> + } >>>> + >>>> + hive->dev_attr = (struct device_attribute) { >>>> + .attr = { >>>> + .name = "xgmi_hive_id", >>>> + .mode = S_IRUGO, >>>> + >>>> + }, >>>> + .show = amdgpu_xgmi_show_hive_id, >>>> + }; >>> Why do you put the attribute into the hive? Usually those are statically >>> declared somewhere. >> To access the hive from amdgpu_xgmi_show_hive_id using container_of > Ok that is rather unusual. > > Can't we use device_create_file() and a proper device attribute here? > > See amdgpu_get_dpm_forced_performance_level for an example how to use it. device_create_file will attach the file to the device's kobj which will place it in sys/class/drm/cardX/device while we want it to reside inside the xgmi_hive_info folder, for this i added kobj to the hive and use sysfs_create_file directly (device_create_file is just a wrapper around this) to specify i want to create the file (device property) for hive's kobj. Andrey > > Christian. > >> Andrey >> >> >>> Apart from that looks good to me, >>> Christian. >>> >>>> + >>>> + ret = sysfs_create_file(hive->kobj, &hive->dev_attr.attr); >>>> + if (ret) { >>>> + dev_err(adev->dev, "XGMI: Failed to create device file xgmi_hive_id\n"); >>>> + kobject_del(hive->kobj); >>>> + kobject_put(hive->kobj); >>>> + hive->kobj = NULL; >>>> + } >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static void amdgpu_xgmi_sysfs_destroy(struct amdgpu_device *adev, >>>> + struct amdgpu_hive_info *hive) >>>> +{ >>>> + sysfs_remove_file(hive->kobj, &hive->dev_attr.attr); >>>> + kobject_del(hive->kobj); >>>> + kobject_put(hive->kobj); >>>> + hive->kobj = NULL; >>>> +} >>>> + >>>> +static ssize_t amdgpu_xgmi_show_device_id(struct device *dev, >>>> + struct device_attribute *attr, >>>> + char *buf) >>>> +{ >>>> + struct drm_device *ddev = dev_get_drvdata(dev); >>>> + struct amdgpu_device *adev = ddev->dev_private; >>>> + >>>> + return snprintf(buf, PAGE_SIZE, "%llu\n", adev->gmc.xgmi.node_id); >>>> + >>>> +} >>>> + >>>> + >>>> +static DEVICE_ATTR(xgmi_device_id, S_IRUGO, amdgpu_xgmi_show_device_id, NULL); >>>> + >>>> + >>>> +static int amdgpu_xgmi_sysf_add_dev_info(struct amdgpu_device *adev, >>>> + struct amdgpu_hive_info *hive) >>>> +{ >>>> + int ret = 0; >>>> + char node[10] = { 0 }; >>>> + >>>> + /* Create xgmi device id file */ >>>> + ret = device_create_file(adev->dev, &dev_attr_xgmi_device_id); >>>> + if (ret) { >>>> + dev_err(adev->dev, "XGMI: Failed to create device file xgmi_device_id\n"); >>>> + return ret; >>>> + } >>>> + >>>> + /* Create sysfs link to hive info folder on the first device */ >>>> + if (adev != hive->adev) { >>>> + ret = sysfs_create_link(&adev->dev->kobj, hive->kobj, >>>> + "xgmi_hive_info"); >>>> + if (ret) { >>>> + dev_err(adev->dev, "XGMI: Failed to create link to hive info"); >>>> + goto remove_file; >>>> + } >>>> + } >>>> + >>>> + sprintf(node, "node%d", hive->number_devices); >>>> + /* Create sysfs link form the hive folder to yorself */ >>>> + ret = sysfs_create_link(hive->kobj, &adev->dev->kobj, node); >>>> + if (ret) { >>>> + dev_err(adev->dev, "XGMI: Failed to create link from hive info"); >>>> + goto remove_link; >>>> + } >>>> + >>>> + goto success; >>>> + >>>> + >>>> +remove_link: >>>> + sysfs_remove_link(&adev->dev->kobj, adev->ddev->unique); >>>> + >>>> +remove_file: >>>> + device_remove_file(adev->dev, &dev_attr_xgmi_device_id); >>>> + >>>> +success: >>>> + return ret; >>>> +} >>>> + >>>> +static void amdgpu_xgmi_sysf_rem_dev_info(struct amdgpu_device *adev, >>>> + struct amdgpu_hive_info *hive) >>>> +{ >>>> + device_remove_file(adev->dev, &dev_attr_xgmi_device_id); >>>> + sysfs_remove_link(&adev->dev->kobj, adev->ddev->unique); >>>> + sysfs_remove_link(hive->kobj, adev->ddev->unique); >>>> +} >>>> + >>>> + >>>> + >>>> struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock) >>>> { >>>> int i; >>>> @@ -66,10 +186,18 @@ struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lo >>>> >>>> /* initialize new hive if not exist */ >>>> tmp = &xgmi_hives[hive_count++]; >>>> + >>>> + if (amdgpu_xgmi_sysfs_create(adev, tmp)) { >>>> + mutex_unlock(&xgmi_mutex); >>>> + return NULL; >>>> + } >>>> + >>>> + tmp->adev = adev; >>>> tmp->hive_id = adev->gmc.xgmi.hive_id; >>>> INIT_LIST_HEAD(&tmp->device_list); >>>> mutex_init(&tmp->hive_lock); >>>> mutex_init(&tmp->reset_lock); >>>> + >>>> if (lock) >>>> mutex_lock(&tmp->hive_lock); >>>> >>>> @@ -156,8 +284,17 @@ int amdgpu_xgmi_add_device(struct amdgpu_device *adev) >>>> break; >>>> } >>>> >>>> - dev_info(adev->dev, "XGMI: Add node %d, hive 0x%llx.\n", >>>> - adev->gmc.xgmi.physical_node_id, adev->gmc.xgmi.hive_id); >>>> + if (!ret) >>>> + ret = amdgpu_xgmi_sysf_add_dev_info(adev, hive); >>>> + >>>> + if (!ret) >>>> + dev_info(adev->dev, "XGMI: Add node %d, hive 0x%llx.\n", >>>> + adev->gmc.xgmi.physical_node_id, adev->gmc.xgmi.hive_id); >>>> + else >>>> + dev_err(adev->dev, "XGMI: Failed to add node %d, hive 0x%llx ret: %d\n", >>>> + adev->gmc.xgmi.physical_node_id, adev->gmc.xgmi.hive_id, >>>> + ret); >>>> + >>>> >>>> mutex_unlock(&hive->hive_lock); >>>> exit: >>>> @@ -176,9 +313,11 @@ void amdgpu_xgmi_remove_device(struct amdgpu_device *adev) >>>> return; >>>> >>>> if (!(hive->number_devices--)) { >>>> + amdgpu_xgmi_sysfs_destroy(adev, hive); >>>> mutex_destroy(&hive->hive_lock); >>>> mutex_destroy(&hive->reset_lock); >>>> } else { >>>> + amdgpu_xgmi_sysf_rem_dev_info(adev, hive); >>>> mutex_unlock(&hive->hive_lock); >>>> } >>>> } >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h >>>> index 14bc606..24a3b03 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h >>>> @@ -29,8 +29,10 @@ struct amdgpu_hive_info { >>>> struct list_head device_list; >>>> struct psp_xgmi_topology_info topology_info; >>>> int number_devices; >>>> - struct mutex hive_lock, >>>> - reset_lock; >>>> + struct mutex hive_lock, reset_lock; >>>> + struct kobject *kobj; >>>> + struct device_attribute dev_attr; >>>> + struct amdgpu_device *adev; >>>> }; >>>> >>>> struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev, int lock); _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx