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

[Liu, Shaoyun]

[Public]

Yes,  we  need the correct setting  in both bare-metal and sriov to populate the correct xgmi  link info.  Move the setting to a common place that not be affect by SRIOV or not make sense to me.  

The change is reviewed by : Shaoyun.liu <Shaoyun.liu at amd.com>


-----Original Message-----
From: Kim, Jonathan <Jonathan.Kim at amd.com> 
Sent: Wednesday, March 9, 2022 6:31 PM
To: Kuehling, Felix <Felix.Kuehling 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

[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).

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)) {