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.grodzov...@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.
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
