[PATCH] drm/amdkfd: add family_id property for kfd_node

[PATCH] drm/amdkfd: add family_id property for kfd_node

[Lang Yu]

Then we can remove the burden of maintaining codes to
parse family_id from gfx version in rocr,
i.e., remove DevIDToAddrLibFamily().

Signed-off-by: Lang Yu <Lang.Yu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 3 +++
 drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 3f0a4a415907..7e0331e853d5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -507,6 +507,8 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr,
 			      dev->node_props.vendor_id);
 	sysfs_show_32bit_prop(buffer, offs, "device_id",
 			      dev->node_props.device_id);
+	sysfs_show_32bit_prop(buffer, offs, "family_id",
+			      dev->node_props.family_id);
 	sysfs_show_32bit_prop(buffer, offs, "location_id",
 			      dev->node_props.location_id);
 	sysfs_show_32bit_prop(buffer, offs, "domain",
@@ -1690,6 +1692,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
 	dev->node_props.gfx_target_version = gpu->device_info.gfx_target_version;
 	dev->node_props.vendor_id = gpu->pdev->vendor;
 	dev->node_props.device_id = gpu->pdev->device;
+	dev->node_props.family_id = gpu->adev->family;
 	dev->node_props.capability |=
 		((dev->gpu->adev->rev_id << HSA_CAP_ASIC_REVISION_SHIFT) &
 			HSA_CAP_ASIC_REVISION_MASK);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
index 9f6c949186c1..4ff8dd2c9549 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
@@ -56,6 +56,7 @@ struct kfd_node_properties {
 	uint32_t gfx_target_version;
 	uint32_t vendor_id;
 	uint32_t device_id;
+	uint32_t family_id;
 	uint32_t location_id;
 	uint32_t domain;
 	uint32_t max_engine_clk_fcompute;
-- 
2.25.1

Re: [PATCH] drm/amdkfd: add family_id property for kfd_node

[Felix Kuehling]

Am 2022-08-16 um 23:09 schrieb Lang Yu:
> Then we can remove the burden of maintaining codes to
> parse family_id from gfx version in rocr,
> i.e., remove DevIDToAddrLibFamily().

I'm OK with the change. But you won't be able to remove 
DevIDToAddrLibFamily as long as ROCr needs to support older kernels that 
don't provide this info.

Regards,
   Felix


>
> Signed-off-by: Lang Yu <Lang.Yu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 3 +++
>   drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 1 +
>   2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index 3f0a4a415907..7e0331e853d5 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -507,6 +507,8 @@ static ssize_t node_show(struct kobject *kobj, struct attribute *attr,
>   			      dev->node_props.vendor_id);
>   	sysfs_show_32bit_prop(buffer, offs, "device_id",
>   			      dev->node_props.device_id);
> +	sysfs_show_32bit_prop(buffer, offs, "family_id",
> +			      dev->node_props.family_id);
>   	sysfs_show_32bit_prop(buffer, offs, "location_id",
>   			      dev->node_props.location_id);
>   	sysfs_show_32bit_prop(buffer, offs, "domain",
> @@ -1690,6 +1692,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
>   	dev->node_props.gfx_target_version = gpu->device_info.gfx_target_version;
>   	dev->node_props.vendor_id = gpu->pdev->vendor;
>   	dev->node_props.device_id = gpu->pdev->device;
> +	dev->node_props.family_id = gpu->adev->family;
>   	dev->node_props.capability |=
>   		((dev->gpu->adev->rev_id << HSA_CAP_ASIC_REVISION_SHIFT) &
>   			HSA_CAP_ASIC_REVISION_MASK);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> index 9f6c949186c1..4ff8dd2c9549 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> @@ -56,6 +56,7 @@ struct kfd_node_properties {
>   	uint32_t gfx_target_version;
>   	uint32_t vendor_id;
>   	uint32_t device_id;
> +	uint32_t family_id;
>   	uint32_t location_id;
>   	uint32_t domain;
>   	uint32_t max_engine_clk_fcompute;

Re: [PATCH] drm/amdkfd: add family_id property for kfd_node

[Felix Kuehling]

Am 2022-08-17 um 11:04 schrieb Felix Kuehling:
> Am 2022-08-16 um 23:09 schrieb Lang Yu:
>> Then we can remove the burden of maintaining codes to
>> parse family_id from gfx version in rocr,
>> i.e., remove DevIDToAddrLibFamily().
>
> I'm OK with the change. But you won't be able to remove 
> DevIDToAddrLibFamily as long as ROCr needs to support older kernels 
> that don't provide this info.
As an alternative you should be able to use the AMDGPU_INFO_DEV_INFO 
ioctl from the render node. The Thunk already has the render nodes of 
all the GPUs. This will work on older kernels as well and will allow you 
to remove DevIDToAddrLibFamily.

Regards,
   Felix


>
> Regards,
>   Felix
>
>
>>
>> Signed-off-by: Lang Yu <Lang.Yu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 3 +++
>>   drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 1 +
>>   2 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> index 3f0a4a415907..7e0331e853d5 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> @@ -507,6 +507,8 @@ static ssize_t node_show(struct kobject *kobj, 
>> struct attribute *attr,
>>                     dev->node_props.vendor_id);
>>       sysfs_show_32bit_prop(buffer, offs, "device_id",
>>                     dev->node_props.device_id);
>> +    sysfs_show_32bit_prop(buffer, offs, "family_id",
>> +                  dev->node_props.family_id);
>>       sysfs_show_32bit_prop(buffer, offs, "location_id",
>>                     dev->node_props.location_id);
>>       sysfs_show_32bit_prop(buffer, offs, "domain",
>> @@ -1690,6 +1692,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
>>       dev->node_props.gfx_target_version = 
>> gpu->device_info.gfx_target_version;
>>       dev->node_props.vendor_id = gpu->pdev->vendor;
>>       dev->node_props.device_id = gpu->pdev->device;
>> +    dev->node_props.family_id = gpu->adev->family;
>>       dev->node_props.capability |=
>>           ((dev->gpu->adev->rev_id << HSA_CAP_ASIC_REVISION_SHIFT) &
>>               HSA_CAP_ASIC_REVISION_MASK);
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h 
>> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>> index 9f6c949186c1..4ff8dd2c9549 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>> @@ -56,6 +56,7 @@ struct kfd_node_properties {
>>       uint32_t gfx_target_version;
>>       uint32_t vendor_id;
>>       uint32_t device_id;
>> +    uint32_t family_id;
>>       uint32_t location_id;
>>       uint32_t domain;
>>       uint32_t max_engine_clk_fcompute;

Re: [PATCH] drm/amdkfd: add family_id property for kfd_node

[Lang Yu]

On 08/17/ , Felix Kuehling wrote:
> Am 2022-08-17 um 11:04 schrieb Felix Kuehling:
> > Am 2022-08-16 um 23:09 schrieb Lang Yu:
> > > Then we can remove the burden of maintaining codes to
> > > parse family_id from gfx version in rocr,
> > > i.e., remove DevIDToAddrLibFamily().
> > 
> > I'm OK with the change. But you won't be able to remove
> > DevIDToAddrLibFamily as long as ROCr needs to support older kernels that
> > don't provide this info.
> As an alternative you should be able to use the AMDGPU_INFO_DEV_INFO ioctl
> from the render node. The Thunk already has the render nodes of all the
> GPUs. This will work on older kernels as well and will allow you to remove
> DevIDToAddrLibFamily.

Yes, topology_get_marketing_name() has called amdgpu_device_initialize()
and got family id info in Thunk. I will put the family id info into
HsaNodeProperties. Thanks for your suggestion.

Regards,
Lang

> Regards,
>   Felix
> 
> 
> > 
> > Regards,
> >   Felix
> > 
> > 
> > > 
> > > Signed-off-by: Lang Yu <Lang.Yu@amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 3 +++
> > >   drivers/gpu/drm/amd/amdkfd/kfd_topology.h | 1 +
> > >   2 files changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > > b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > > index 3f0a4a415907..7e0331e853d5 100644
> > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > > @@ -507,6 +507,8 @@ static ssize_t node_show(struct kobject *kobj,
> > > struct attribute *attr,
> > >                     dev->node_props.vendor_id);
> > >       sysfs_show_32bit_prop(buffer, offs, "device_id",
> > >                     dev->node_props.device_id);
> > > +    sysfs_show_32bit_prop(buffer, offs, "family_id",
> > > +                  dev->node_props.family_id);
> > >       sysfs_show_32bit_prop(buffer, offs, "location_id",
> > >                     dev->node_props.location_id);
> > >       sysfs_show_32bit_prop(buffer, offs, "domain",
> > > @@ -1690,6 +1692,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
> > >       dev->node_props.gfx_target_version =
> > > gpu->device_info.gfx_target_version;
> > >       dev->node_props.vendor_id = gpu->pdev->vendor;
> > >       dev->node_props.device_id = gpu->pdev->device;
> > > +    dev->node_props.family_id = gpu->adev->family;
> > >       dev->node_props.capability |=
> > >           ((dev->gpu->adev->rev_id << HSA_CAP_ASIC_REVISION_SHIFT) &
> > >               HSA_CAP_ASIC_REVISION_MASK);
> > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > > b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > > index 9f6c949186c1..4ff8dd2c9549 100644
> > > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
> > > @@ -56,6 +56,7 @@ struct kfd_node_properties {
> > >       uint32_t gfx_target_version;
> > >       uint32_t vendor_id;
> > >       uint32_t device_id;
> > > +    uint32_t family_id;
> > >       uint32_t location_id;
> > >       uint32_t domain;
> > >       uint32_t max_engine_clk_fcompute;