[PATCH] drm/amdgpu: fix aldebaran xgmi topology for vf

[Felix Kuehling]

On 2022-03-09 18:31, Kim, Jonathan wrote:
> [Public]
>
>> -----Original Message-----
>> From: Kuehling, Felix <Felix.Kuehling at amd.com>
>> Sent: March 9, 2022 6:12 PM
>> To: Kim, Jonathan <Jonathan.Kim at amd.com>; amd-gfx at lists.freedesktop.org
>> Cc: Liu, Shaoyun <Shaoyun.Liu at amd.com>
>> Subject: Re: [PATCH] drm/amdgpu: fix aldebaran xgmi topology for vf
>>
>> On 2022-03-09 17:16, Jonathan Kim wrote:
>>> VFs must also distinguish whether or not the TA supports full duplex
>>> or half duplex link records in order to report the correct xGMI topology.
>>>
>>> Signed-off-by: Jonathan Kim <jonathan.kim at amd.com>
>> I think I'm missing something here. Your condition for setting
>> supports_extended_data is exactly the same, but you're initializing it in a
>> different function. Can you explain how that change relates to SRIOV?
> I probably should have included more context when sending this out.
> The proposed support assignment happens after this:
>
>          if (amdgpu_sriov_vf(adev))
>                  ret = psp_init_sriov_microcode(psp);
>          else
>                  ret = psp_init_microcode(psp);
>          if (ret) {
>                  DRM_ERROR("Failed to load psp firmware!\n");
>                  return ret;
>          }
>
> and psp_init_sriov_microde doesn't set secure OS micro code info (this is where the support assignment currently is).

I see, so psp_init_sos_base_fw doesn't run on VFs and therefore the XGMI 
info wasn't correct. Moving this to psp_sw_init puts it in a common 
place that is executed on VFs and bare metal. Makes sense.

If you add that explanation to the patch description, the patch is

Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>


>
> Thanks,
>
> Jon
>
>> Thanks,
>>     Felix
>>
>>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>> index 3ce1d38a7822..a6acec1a6155 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>> @@ -310,6 +310,10 @@ static int psp_sw_init(void *handle)
>>>              return ret;
>>>      }
>>>
>>> +   adev->psp.xgmi_context.supports_extended_data =
>>> +           !adev->gmc.xgmi.connected_to_cpu &&
>>> +                   adev->ip_versions[MP0_HWIP][0] == IP_VERSION(13,
>> 0, 2);
>>> +
>>>      memset(&boot_cfg_entry, 0, sizeof(boot_cfg_entry));
>>>      if (psp_get_runtime_db_entry(adev,
>>>                              PSP_RUNTIME_ENTRY_TYPE_BOOT_CONFIG,
>>> @@ -3008,7 +3012,6 @@ static int psp_init_sos_base_fw(struct
>> amdgpu_device *adev)
>>>              adev->psp.sos.size_bytes = le32_to_cpu(sos_hdr-
>>> sos.size_bytes);
>>>              adev->psp.sos.start_addr = ucode_array_start_addr +
>>>                              le32_to_cpu(sos_hdr->sos.offset_bytes);
>>> -           adev->psp.xgmi_context.supports_extended_data = false;
>>>      } else {
>>>              /* Load alternate PSP SOS FW */
>>>              sos_hdr_v1_3 = (const struct psp_firmware_header_v1_3
>>> *)adev->psp.sos_fw->data; @@ -3023,7 +3026,6 @@ static int
>> psp_init_sos_base_fw(struct amdgpu_device *adev)
>>>              adev->psp.sos.size_bytes = le32_to_cpu(sos_hdr_v1_3-
>>> sos_aux.size_bytes);
>>>              adev->psp.sos.start_addr = ucode_array_start_addr +
>>>                      le32_to_cpu(sos_hdr_v1_3->sos_aux.offset_bytes);
>>> -           adev->psp.xgmi_context.supports_extended_data = true;
>>>      }
>>>
>>>      if ((adev->psp.sys.size_bytes == 0) || (adev->psp.sos.size_bytes ==
>>> 0)) {