[PATCH] drm/amdkfd: Align unique_id format to match amdgpu

[PATCH] drm/amdkfd: Align unique_id format to match amdgpu

[Kent Russell]

unique_id is printed as %016llx in amdgpu, but %llu in KFD. Call the
sysfs_show_gen_prop function directly and use the %016llx format, to
align with amdgpu. Don't need to add a new macro since this is a one-off.

Signed-off-by: Kent Russell <kent.russell@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index c8c75ff7cea8..4dac29cdab20 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -538,7 +538,7 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr,
 				      dev->node_props.debug_prop);
 		sysfs_show_32bit_prop(buffer, offs, "sdma_fw_version",
 				      dev->gpu->kfd->sdma_fw_version);
-		sysfs_show_64bit_prop(buffer, offs, "unique_id",
+		sysfs_show_gen_prop(buffer, offs, "%s %016llx\n", "unique_id",
 				      dev->gpu->adev->unique_id);
 		sysfs_show_32bit_prop(buffer, offs, "num_xcc",
 				      NUM_XCC(dev->gpu->xcc_mask));
-- 
2.34.1

Re: [PATCH] drm/amdkfd: Align unique_id format to match amdgpu

[Felix Kuehling]

On 2023-09-14 13:09, Kent Russell wrote:
> unique_id is printed as %016llx in amdgpu, but %llu in KFD. Call the
> sysfs_show_gen_prop function directly and use the %016llx format, to
> align with amdgpu. Don't need to add a new macro since this is a one-off.

Doesn't this break the ABI? Any tool currently reading the unique ID 
would expect it to be decimal.

Regards,
   Felix


>
> Signed-off-by: Kent Russell <kent.russell@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index c8c75ff7cea8..4dac29cdab20 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -538,7 +538,7 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr,
>   				      dev->node_props.debug_prop);
>   		sysfs_show_32bit_prop(buffer, offs, "sdma_fw_version",
>   				      dev->gpu->kfd->sdma_fw_version);
> -		sysfs_show_64bit_prop(buffer, offs, "unique_id",
> +		sysfs_show_gen_prop(buffer, offs, "%s %016llx\n", "unique_id",
>   				      dev->gpu->adev->unique_id);
>   		sysfs_show_32bit_prop(buffer, offs, "num_xcc",
>   				      NUM_XCC(dev->gpu->xcc_mask));

RE: [PATCH] drm/amdkfd: Align unique_id format to match amdgpu

[Russell, Kent]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Kuehling, Felix <Felix.Kuehling@amd.com>
> Sent: Thursday, September 14, 2023 1:25 PM
> To: Russell, Kent <Kent.Russell@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdkfd: Align unique_id format to match amdgpu
>
>
> On 2023-09-14 13:09, Kent Russell wrote:
> > unique_id is printed as %016llx in amdgpu, but %llu in KFD. Call the
> > sysfs_show_gen_prop function directly and use the %016llx format, to
> > align with amdgpu. Don't need to add a new macro since this is a one-off.
>
> Doesn't this break the ABI? Any tool currently reading the unique ID
> would expect it to be decimal.

A fair point, and was something I was on the fence on. The inquiry came from rocm-smi, noting that the unique_ids didn't align. One would assume that reading a sysfs file would be done as a string and then converted, but you're right. We can't risk breaking other tools just because we want to fix one of ours and make assumptions about how tools consume the information. I'll talk to the SMI guys about trying to convert to hex in SMI to align instead.

 Kent

>
> Regards,
>    Felix
>
>
> >
> > Signed-off-by: Kent Russell <kent.russell@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > index c8c75ff7cea8..4dac29cdab20 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > @@ -538,7 +538,7 @@ static ssize_t node_show(struct kobject *kobj, struct
> attribute *attr,
> >                                   dev->node_props.debug_prop);
> >             sysfs_show_32bit_prop(buffer, offs, "sdma_fw_version",
> >                                   dev->gpu->kfd->sdma_fw_version);
> > -           sysfs_show_64bit_prop(buffer, offs, "unique_id",
> > +           sysfs_show_gen_prop(buffer, offs, "%s %016llx\n", "unique_id",
> >                                   dev->gpu->adev->unique_id);
> >             sysfs_show_32bit_prop(buffer, offs, "num_xcc",
> >                                   NUM_XCC(dev->gpu->xcc_mask));