* [PATCH 1/2] drm/amdgpu: cleanup amdgpu_xgmi_sysfs_add_dev_info
@ 2022-01-26 11:59 Christian König
2022-01-26 11:59 ` [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id Christian König
2022-01-26 12:50 ` [PATCH 1/2] drm/amdgpu: cleanup amdgpu_xgmi_sysfs_add_dev_info Luben Tuikov
0 siblings, 2 replies; 6+ messages in thread
From: Christian König @ 2022-01-26 11:59 UTC (permalink / raw)
To: Luben.Tuikov, Tom.StDenis; +Cc: amd-gfx
Don't initialize variables if it isn't absolutely necessary.
Use amdgpu_xgmi_sysfs_rem_dev_info to cleanup when something goes wrong.
Drop the explicit warnings since the sysfs core warns about things like
duplicate files itself.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 85 +++++++++---------------
1 file changed, 33 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 5929d6f528c9..68509f619ba3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -289,61 +289,10 @@ static ssize_t amdgpu_xgmi_show_error(struct device *dev,
static DEVICE_ATTR(xgmi_device_id, S_IRUGO, amdgpu_xgmi_show_device_id, NULL);
static DEVICE_ATTR(xgmi_error, S_IRUGO, amdgpu_xgmi_show_error, NULL);
-static int amdgpu_xgmi_sysfs_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 xgmi error file */
- ret = device_create_file(adev->dev, &dev_attr_xgmi_error);
- if (ret)
- pr_err("failed to create xgmi_error\n");
-
-
- /* Create sysfs link to hive info folder on the first device */
- if (hive->kobj.parent != (&adev->dev->kobj)) {
- 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", atomic_read(&hive->number_devices));
- /* Create sysfs link form the hive folder to yourself */
- 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_to_drm(adev)->unique);
-
-remove_file:
- device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
-
-success:
- return ret;
-}
-
static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
struct amdgpu_hive_info *hive)
{
char node[10];
- memset(node, 0, sizeof(node));
device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
device_remove_file(adev->dev, &dev_attr_xgmi_error);
@@ -353,10 +302,42 @@ static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
sprintf(node, "node%d", atomic_read(&hive->number_devices));
sysfs_remove_link(&hive->kobj, node);
-
}
+static int amdgpu_xgmi_sysfs_add_dev_info(struct amdgpu_device *adev,
+ struct amdgpu_hive_info *hive)
+{
+ char node[10];
+ int r;
+
+ r = device_create_file(adev->dev, &dev_attr_xgmi_device_id);
+ if (r)
+ return r;
+
+ r = device_create_file(adev->dev, &dev_attr_xgmi_error);
+ if (r)
+ goto error;
+ /* Create sysfs link to hive info folder on the first device */
+ if (hive->kobj.parent != (&adev->dev->kobj)) {
+ r = sysfs_create_link(&adev->dev->kobj, &hive->kobj,
+ "xgmi_hive_info");
+ if (r)
+ goto error;
+ }
+
+ /* Create sysfs link form the hive folder to yourself */
+ sprintf(node, "node%d", atomic_read(&hive->number_devices));
+ r = sysfs_create_link(&hive->kobj, &adev->dev->kobj, node);
+ if (r)
+ goto error;
+
+ return 0;
+
+error:
+ amdgpu_xgmi_sysfs_rem_dev_info(adev, hive);
+ return r;
+}
struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id
2022-01-26 11:59 [PATCH 1/2] drm/amdgpu: cleanup amdgpu_xgmi_sysfs_add_dev_info Christian König
@ 2022-01-26 11:59 ` Christian König
2022-01-26 12:55 ` Luben Tuikov
2022-01-26 12:50 ` [PATCH 1/2] drm/amdgpu: cleanup amdgpu_xgmi_sysfs_add_dev_info Luben Tuikov
1 sibling, 1 reply; 6+ messages in thread
From: Christian König @ 2022-01-26 11:59 UTC (permalink / raw)
To: Luben.Tuikov, Tom.StDenis; +Cc: amd-gfx
umr needs that to correctly calculate the VRAM base address
inside the MC address space.
Only compile tested!
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 34 ++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 68509f619ba3..21a5d07a1abf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -252,6 +252,26 @@ static ssize_t amdgpu_xgmi_show_device_id(struct device *dev,
}
+static ssize_t amdgpu_xgmi_show_node_segment_size(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_device *ddev = dev_get_drvdata(dev);
+ struct amdgpu_device *adev = drm_to_adev(ddev);
+
+ return sysfs_emit(buf, "%llu\n", adev->gmc.xgmi.node_segment_size);
+}
+
+static ssize_t amdgpu_xgmi_show_physical_node_id(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_device *ddev = dev_get_drvdata(dev);
+ struct amdgpu_device *adev = drm_to_adev(ddev);
+
+ return sysfs_emit(buf, "%u\n", adev->gmc.xgmi.physical_node_id);
+}
+
#define AMDGPU_XGMI_SET_FICAA(o) ((o) | 0x456801)
static ssize_t amdgpu_xgmi_show_error(struct device *dev,
struct device_attribute *attr,
@@ -287,6 +307,10 @@ static ssize_t amdgpu_xgmi_show_error(struct device *dev,
static DEVICE_ATTR(xgmi_device_id, S_IRUGO, amdgpu_xgmi_show_device_id, NULL);
+static DEVICE_ATTR(xgmi_node_segment_size, S_IRUGO,
+ amdgpu_xgmi_show_node_segment_size, NULL);
+static DEVICE_ATTR(xgmi_physical_node_id, S_IRUGO,
+ amdgpu_xgmi_show_physical_node_id, NULL);
static DEVICE_ATTR(xgmi_error, S_IRUGO, amdgpu_xgmi_show_error, NULL);
static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
@@ -295,6 +319,8 @@ static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
char node[10];
device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
+ device_remove_file(adev->dev, &dev_attr_xgmi_node_segment_size);
+ device_remove_file(adev->dev, &dev_attr_xgmi_physical_node_id);
device_remove_file(adev->dev, &dev_attr_xgmi_error);
if (hive->kobj.parent != (&adev->dev->kobj))
@@ -318,6 +344,14 @@ static int amdgpu_xgmi_sysfs_add_dev_info(struct amdgpu_device *adev,
if (r)
goto error;
+ r = device_create_file(adev->dev, &dev_attr_xgmi_node_segment_size);
+ if (r)
+ goto error;
+
+ r = device_create_file(adev->dev, &dev_attr_xgmi_physical_node_id);
+ if (r)
+ goto error;
+
/* Create sysfs link to hive info folder on the first device */
if (hive->kobj.parent != (&adev->dev->kobj)) {
r = sysfs_create_link(&adev->dev->kobj, &hive->kobj,
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id
2022-01-26 11:59 ` [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id Christian König
@ 2022-01-26 12:55 ` Luben Tuikov
2022-01-26 12:57 ` StDenis, Tom
0 siblings, 1 reply; 6+ messages in thread
From: Luben Tuikov @ 2022-01-26 12:55 UTC (permalink / raw)
To: Christian König, Tom.StDenis; +Cc: amd-gfx
This seems reasonable. Hope it works out for umr.
Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
Regards,
Luben
On 2022-01-26 06:59, Christian König wrote:
> umr needs that to correctly calculate the VRAM base address
> inside the MC address space.
>
> Only compile tested!
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 34 ++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 68509f619ba3..21a5d07a1abf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -252,6 +252,26 @@ static ssize_t amdgpu_xgmi_show_device_id(struct device *dev,
>
> }
>
> +static ssize_t amdgpu_xgmi_show_node_segment_size(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(ddev);
> +
> + return sysfs_emit(buf, "%llu\n", adev->gmc.xgmi.node_segment_size);
> +}
> +
> +static ssize_t amdgpu_xgmi_show_physical_node_id(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(ddev);
> +
> + return sysfs_emit(buf, "%u\n", adev->gmc.xgmi.physical_node_id);
> +}
> +
> #define AMDGPU_XGMI_SET_FICAA(o) ((o) | 0x456801)
> static ssize_t amdgpu_xgmi_show_error(struct device *dev,
> struct device_attribute *attr,
> @@ -287,6 +307,10 @@ static ssize_t amdgpu_xgmi_show_error(struct device *dev,
>
>
> static DEVICE_ATTR(xgmi_device_id, S_IRUGO, amdgpu_xgmi_show_device_id, NULL);
> +static DEVICE_ATTR(xgmi_node_segment_size, S_IRUGO,
> + amdgpu_xgmi_show_node_segment_size, NULL);
> +static DEVICE_ATTR(xgmi_physical_node_id, S_IRUGO,
> + amdgpu_xgmi_show_physical_node_id, NULL);
> static DEVICE_ATTR(xgmi_error, S_IRUGO, amdgpu_xgmi_show_error, NULL);
>
> static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
> @@ -295,6 +319,8 @@ static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
> char node[10];
>
> device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
> + device_remove_file(adev->dev, &dev_attr_xgmi_node_segment_size);
> + device_remove_file(adev->dev, &dev_attr_xgmi_physical_node_id);
> device_remove_file(adev->dev, &dev_attr_xgmi_error);
>
> if (hive->kobj.parent != (&adev->dev->kobj))
> @@ -318,6 +344,14 @@ static int amdgpu_xgmi_sysfs_add_dev_info(struct amdgpu_device *adev,
> if (r)
> goto error;
>
> + r = device_create_file(adev->dev, &dev_attr_xgmi_node_segment_size);
> + if (r)
> + goto error;
> +
> + r = device_create_file(adev->dev, &dev_attr_xgmi_physical_node_id);
> + if (r)
> + goto error;
> +
> /* Create sysfs link to hive info folder on the first device */
> if (hive->kobj.parent != (&adev->dev->kobj)) {
> r = sysfs_create_link(&adev->dev->kobj, &hive->kobj,
Regards,
--
Luben
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id
2022-01-26 12:55 ` Luben Tuikov
@ 2022-01-26 12:57 ` StDenis, Tom
2022-02-09 8:51 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: StDenis, Tom @ 2022-01-26 12:57 UTC (permalink / raw)
To: Tuikov, Luben, Christian König; +Cc: amd-gfx
[AMD Official Use Only]
Sadly I don't control any XGMI hosts to try it out. So if they pick it up in their builds I can but otherwise we'll have to wait.
Tom
________________________________________
From: Tuikov, Luben <Luben.Tuikov@amd.com>
Sent: Wednesday, January 26, 2022 07:55
To: Christian König; StDenis, Tom
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id
This seems reasonable. Hope it works out for umr.
Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
Regards,
Luben
On 2022-01-26 06:59, Christian König wrote:
> umr needs that to correctly calculate the VRAM base address
> inside the MC address space.
>
> Only compile tested!
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 34 ++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 68509f619ba3..21a5d07a1abf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -252,6 +252,26 @@ static ssize_t amdgpu_xgmi_show_device_id(struct device *dev,
>
> }
>
> +static ssize_t amdgpu_xgmi_show_node_segment_size(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(ddev);
> +
> + return sysfs_emit(buf, "%llu\n", adev->gmc.xgmi.node_segment_size);
> +}
> +
> +static ssize_t amdgpu_xgmi_show_physical_node_id(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(ddev);
> +
> + return sysfs_emit(buf, "%u\n", adev->gmc.xgmi.physical_node_id);
> +}
> +
> #define AMDGPU_XGMI_SET_FICAA(o) ((o) | 0x456801)
> static ssize_t amdgpu_xgmi_show_error(struct device *dev,
> struct device_attribute *attr,
> @@ -287,6 +307,10 @@ static ssize_t amdgpu_xgmi_show_error(struct device *dev,
>
>
> static DEVICE_ATTR(xgmi_device_id, S_IRUGO, amdgpu_xgmi_show_device_id, NULL);
> +static DEVICE_ATTR(xgmi_node_segment_size, S_IRUGO,
> + amdgpu_xgmi_show_node_segment_size, NULL);
> +static DEVICE_ATTR(xgmi_physical_node_id, S_IRUGO,
> + amdgpu_xgmi_show_physical_node_id, NULL);
> static DEVICE_ATTR(xgmi_error, S_IRUGO, amdgpu_xgmi_show_error, NULL);
>
> static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
> @@ -295,6 +319,8 @@ static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
> char node[10];
>
> device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
> + device_remove_file(adev->dev, &dev_attr_xgmi_node_segment_size);
> + device_remove_file(adev->dev, &dev_attr_xgmi_physical_node_id);
> device_remove_file(adev->dev, &dev_attr_xgmi_error);
>
> if (hive->kobj.parent != (&adev->dev->kobj))
> @@ -318,6 +344,14 @@ static int amdgpu_xgmi_sysfs_add_dev_info(struct amdgpu_device *adev,
> if (r)
> goto error;
>
> + r = device_create_file(adev->dev, &dev_attr_xgmi_node_segment_size);
> + if (r)
> + goto error;
> +
> + r = device_create_file(adev->dev, &dev_attr_xgmi_physical_node_id);
> + if (r)
> + goto error;
> +
> /* Create sysfs link to hive info folder on the first device */
> if (hive->kobj.parent != (&adev->dev->kobj)) {
> r = sysfs_create_link(&adev->dev->kobj, &hive->kobj,
Regards,
--
Luben
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id
2022-01-26 12:57 ` StDenis, Tom
@ 2022-02-09 8:51 ` Christian König
0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2022-02-09 8:51 UTC (permalink / raw)
To: StDenis, Tom, Andrey Grodzovsky; +Cc: Tuikov, Luben, amd-gfx
Can anybody give me a Tested-by for this set?
I would really like to push it, but it would be nice to have at least
somebody with access to an xgmi system tries it first.
Christian.
Am 26.01.22 um 13:57 schrieb StDenis, Tom:
> [AMD Official Use Only]
>
> Sadly I don't control any XGMI hosts to try it out. So if they pick it up in their builds I can but otherwise we'll have to wait.
>
> Tom
>
> ________________________________________
> From: Tuikov, Luben <Luben.Tuikov@amd.com>
> Sent: Wednesday, January 26, 2022 07:55
> To: Christian König; StDenis, Tom
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id
>
> This seems reasonable. Hope it works out for umr.
>
> Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
>
> Regards,
> Luben
>
> On 2022-01-26 06:59, Christian König wrote:
>> umr needs that to correctly calculate the VRAM base address
>> inside the MC address space.
>>
>> Only compile tested!
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 34 ++++++++++++++++++++++++
>> 1 file changed, 34 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> index 68509f619ba3..21a5d07a1abf 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
>> @@ -252,6 +252,26 @@ static ssize_t amdgpu_xgmi_show_device_id(struct device *dev,
>>
>> }
>>
>> +static ssize_t amdgpu_xgmi_show_node_segment_size(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct drm_device *ddev = dev_get_drvdata(dev);
>> + struct amdgpu_device *adev = drm_to_adev(ddev);
>> +
>> + return sysfs_emit(buf, "%llu\n", adev->gmc.xgmi.node_segment_size);
>> +}
>> +
>> +static ssize_t amdgpu_xgmi_show_physical_node_id(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct drm_device *ddev = dev_get_drvdata(dev);
>> + struct amdgpu_device *adev = drm_to_adev(ddev);
>> +
>> + return sysfs_emit(buf, "%u\n", adev->gmc.xgmi.physical_node_id);
>> +}
>> +
>> #define AMDGPU_XGMI_SET_FICAA(o) ((o) | 0x456801)
>> static ssize_t amdgpu_xgmi_show_error(struct device *dev,
>> struct device_attribute *attr,
>> @@ -287,6 +307,10 @@ static ssize_t amdgpu_xgmi_show_error(struct device *dev,
>>
>>
>> static DEVICE_ATTR(xgmi_device_id, S_IRUGO, amdgpu_xgmi_show_device_id, NULL);
>> +static DEVICE_ATTR(xgmi_node_segment_size, S_IRUGO,
>> + amdgpu_xgmi_show_node_segment_size, NULL);
>> +static DEVICE_ATTR(xgmi_physical_node_id, S_IRUGO,
>> + amdgpu_xgmi_show_physical_node_id, NULL);
>> static DEVICE_ATTR(xgmi_error, S_IRUGO, amdgpu_xgmi_show_error, NULL);
>>
>> static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
>> @@ -295,6 +319,8 @@ static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
>> char node[10];
>>
>> device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
>> + device_remove_file(adev->dev, &dev_attr_xgmi_node_segment_size);
>> + device_remove_file(adev->dev, &dev_attr_xgmi_physical_node_id);
>> device_remove_file(adev->dev, &dev_attr_xgmi_error);
>>
>> if (hive->kobj.parent != (&adev->dev->kobj))
>> @@ -318,6 +344,14 @@ static int amdgpu_xgmi_sysfs_add_dev_info(struct amdgpu_device *adev,
>> if (r)
>> goto error;
>>
>> + r = device_create_file(adev->dev, &dev_attr_xgmi_node_segment_size);
>> + if (r)
>> + goto error;
>> +
>> + r = device_create_file(adev->dev, &dev_attr_xgmi_physical_node_id);
>> + if (r)
>> + goto error;
>> +
>> /* Create sysfs link to hive info folder on the first device */
>> if (hive->kobj.parent != (&adev->dev->kobj)) {
>> r = sysfs_create_link(&adev->dev->kobj, &hive->kobj,
> Regards,
> --
> Luben
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu: cleanup amdgpu_xgmi_sysfs_add_dev_info
2022-01-26 11:59 [PATCH 1/2] drm/amdgpu: cleanup amdgpu_xgmi_sysfs_add_dev_info Christian König
2022-01-26 11:59 ` [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id Christian König
@ 2022-01-26 12:50 ` Luben Tuikov
1 sibling, 0 replies; 6+ messages in thread
From: Luben Tuikov @ 2022-01-26 12:50 UTC (permalink / raw)
To: Christian König, Tom.StDenis; +Cc: amd-gfx
Yeah, that's cleaner.
Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>
Regards,
Luben
On 2022-01-26 06:59, Christian König wrote:
> Don't initialize variables if it isn't absolutely necessary.
>
> Use amdgpu_xgmi_sysfs_rem_dev_info to cleanup when something goes wrong.
>
> Drop the explicit warnings since the sysfs core warns about things like
> duplicate files itself.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 85 +++++++++---------------
> 1 file changed, 33 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 5929d6f528c9..68509f619ba3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -289,61 +289,10 @@ static ssize_t amdgpu_xgmi_show_error(struct device *dev,
> static DEVICE_ATTR(xgmi_device_id, S_IRUGO, amdgpu_xgmi_show_device_id, NULL);
> static DEVICE_ATTR(xgmi_error, S_IRUGO, amdgpu_xgmi_show_error, NULL);
>
> -static int amdgpu_xgmi_sysfs_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 xgmi error file */
> - ret = device_create_file(adev->dev, &dev_attr_xgmi_error);
> - if (ret)
> - pr_err("failed to create xgmi_error\n");
> -
> -
> - /* Create sysfs link to hive info folder on the first device */
> - if (hive->kobj.parent != (&adev->dev->kobj)) {
> - 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", atomic_read(&hive->number_devices));
> - /* Create sysfs link form the hive folder to yourself */
> - 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_to_drm(adev)->unique);
> -
> -remove_file:
> - device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
> -
> -success:
> - return ret;
> -}
> -
> static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
> struct amdgpu_hive_info *hive)
> {
> char node[10];
> - memset(node, 0, sizeof(node));
>
> device_remove_file(adev->dev, &dev_attr_xgmi_device_id);
> device_remove_file(adev->dev, &dev_attr_xgmi_error);
> @@ -353,10 +302,42 @@ static void amdgpu_xgmi_sysfs_rem_dev_info(struct amdgpu_device *adev,
>
> sprintf(node, "node%d", atomic_read(&hive->number_devices));
> sysfs_remove_link(&hive->kobj, node);
> -
> }
>
> +static int amdgpu_xgmi_sysfs_add_dev_info(struct amdgpu_device *adev,
> + struct amdgpu_hive_info *hive)
> +{
> + char node[10];
> + int r;
> +
> + r = device_create_file(adev->dev, &dev_attr_xgmi_device_id);
> + if (r)
> + return r;
> +
> + r = device_create_file(adev->dev, &dev_attr_xgmi_error);
> + if (r)
> + goto error;
>
> + /* Create sysfs link to hive info folder on the first device */
> + if (hive->kobj.parent != (&adev->dev->kobj)) {
> + r = sysfs_create_link(&adev->dev->kobj, &hive->kobj,
> + "xgmi_hive_info");
> + if (r)
> + goto error;
> + }
> +
> + /* Create sysfs link form the hive folder to yourself */
> + sprintf(node, "node%d", atomic_read(&hive->number_devices));
> + r = sysfs_create_link(&hive->kobj, &adev->dev->kobj, node);
> + if (r)
> + goto error;
> +
> + return 0;
> +
> +error:
> + amdgpu_xgmi_sysfs_rem_dev_info(adev, hive);
> + return r;
> +}
>
> struct amdgpu_hive_info *amdgpu_get_xgmi_hive(struct amdgpu_device *adev)
> {
Regards,
--
Luben
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-09 8:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26 11:59 [PATCH 1/2] drm/amdgpu: cleanup amdgpu_xgmi_sysfs_add_dev_info Christian König
2022-01-26 11:59 ` [PATCH 2/2] drm/amdgpu: add sysfs files for XGMI segment size and physical node id Christian König
2022-01-26 12:55 ` Luben Tuikov
2022-01-26 12:57 ` StDenis, Tom
2022-02-09 8:51 ` Christian König
2022-01-26 12:50 ` [PATCH 1/2] drm/amdgpu: cleanup amdgpu_xgmi_sysfs_add_dev_info Luben Tuikov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.