[PATCH 3/4] drm/amdgpu: Initialize xgmi info during discovery

[Lazar, Lijo]

On 2/7/2025 5:03 AM, Kim, Jonathan wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>> Sent: Thursday, February 6, 2025 8:13 AM
>> To: amd-gfx at lists.freedesktop.org; Lazar, Lijo <Lijo.Lazar at amd.com>
>> Cc: Zhang, Hawking <Hawking.Zhang at amd.com>; Kim, Jonathan
>> <Jonathan.Kim at amd.com>
>> Subject: [PATCH 3/4] drm/amdgpu: Initialize xgmi info during discovery
>>
>> Initialize xgmi related static information during discovery.
>>
>> Signed-off-by: Lijo Lazar <lijo.lazar at amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 20 +++++++++++++------
>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> index eca431e52038..d4eade2bd4d3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>> @@ -2502,6 +2502,19 @@ static int amdgpu_discovery_set_isp_ip_blocks(struct
>> amdgpu_device *adev)
>>       return 0;
>>  }
>>
>> +static void amdgpu_discovery_set_xgmi_info(struct amdgpu_device *adev)
>> +{
>> +     if (amdgpu_ip_version(adev, XGMI_HWIP, 0) == IP_VERSION(4, 8, 0))
>> +             adev->gmc.xgmi.supported = true;
>> +
>> +     if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 3) ||
>> +         amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 4))
>> +             adev->ip_versions[XGMI_HWIP][0] = IP_VERSION(6, 4, 0);
> 
> Can this stuff get rolled into xgm_init_info and called directly into amdgpu_discovery_set_ip_blocks?
> Breaking up discovery_set_xgmi_info and xgmi_init_info as 2 separate things seems a little confusing.
> 

Intent is like this -
	Set IP version info. This is the job of discovery and kept inside
amdgpu_discovery.
	Set any static information derived out of IP versions and not available
in discovery tables. This is kept outside of discovery file.

Thanks,
Lijo

>> +
>> +     if (amdgpu_ip_version(adev, XGMI_HWIP, 0))
> 
> Maybe roll this check into xgmi_init_info i.e. void early return if null.
> 
>> +             amdgpu_xgmi_init_info(adev);
>> +}
>> +
>>  int amdgpu_discovery_set_ip_blocks(struct amdgpu_device *adev)
>>  {
>>       int r;
>> @@ -2769,12 +2782,7 @@ int amdgpu_discovery_set_ip_blocks(struct
>> amdgpu_device *adev)
>>               break;
>>       }
>>
>> -     if (amdgpu_ip_version(adev, XGMI_HWIP, 0) == IP_VERSION(4, 8, 0))
>> -             adev->gmc.xgmi.supported = true;
>> -
>> -     if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 3) ||
>> -         amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 4))
>> -             adev->ip_versions[XGMI_HWIP][0] = IP_VERSION(6, 4, 0);
>> +     amdgpu_discovery_set_xgmi_info(adev);
> 
> If you do the suggestions above, you can just call amdgpu_xgmi_init_info unconditionally.
> 
> Jon
>>
>>       /* set NBIO version */
>>       switch (amdgpu_ip_version(adev, NBIO_HWIP, 0)) {
>> --
>> 2.25.1
>