[PATCH] drm/amdgpu: Add sysfs entries for xgmi hive.
Koenig, Christian
Christian.Koenig at amd.com
Tue Mar 5 18:26:37 UTC 2019
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 at amd.com>
>>> ---
>>> 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.
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);
More information about the amd-gfx
mailing list