[PATCH] drm/amdgpu: Add sysfs entries for xgmi hive.

Grodzovsky, Andrey Andrey.Grodzovsky at amd.com
Tue Mar 5 18:24:11 UTC 2019


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

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