[PATCH 07/13] drm/amdgpu: Add place holder functions for xgmi topology interface with psp

[Christian König]

Well, exactly that's my point. How certain are we that the PSP firmware 
interface is not going to change?

At bare minimum we should add big "/* TODOD: .. */" markers all around 
explaining why those functions aren't implemented yet.

Thanks,
Christian.

Am 07.09.2018 um 08:03 schrieb Kuehling, Felix:
> This is on purpose. These functions for now are place holders because the PSP firmware interface is not ready yet, and we want to start testing XGMI with higher level code with some hard-coded topology. Once we have proper PSP firmware, these place holders will be filled in.
>
> Regards,
>    Felix
>
> ________________________________________
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of Christian König <ckoenig.leichtzumerken at gmail.com>
> Sent: Thursday, September 6, 2018 3:42:33 AM
> To: Liu, Shaoyun; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 07/13] drm/amdgpu: Add place holder functions for xgmi topology interface with psp
>
> Am 05.09.2018 um 17:30 schrieb shaoyunl:
>> From: Shaoyun Liu <Shaoyun.Liu at amd.com>
>>
>> Add dummy function for xgmi function interface with psp
>>
>> Change-Id: I01f35baf5a4b96e9654d448c9892be3cd72c05b7
>> Signed-off-by: Shaoyun Liu <Shaoyun.Liu at amd.com>
>> Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>
> Well NAK. We usually don't add dummy functions and that completely
> circumvents the error handling added in patch #6.
>
> Either we should implement the functions or not call them in the first
> place.
>
> Christian.
>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 26 ++++++++++++++++++++++++++
>>    1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>> index b70cfa3..b1c0b33 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
>> @@ -548,6 +548,29 @@ static int psp_v11_0_mode1_reset(struct psp_context *psp)
>>        return 0;
>>    }
>>
>> +static int psp_v11_0_xgmi_get_topology_info(struct psp_context *psp,
>> +     int number_devices, struct psp_xgmi_topology_info *topology)
>> +{
>> +     return 0;
>> +}
>> +
>> +static int psp_v11_0_xgmi_set_topology_info(struct psp_context *psp,
>> +     int number_devices, struct psp_xgmi_topology_info *topology)
>> +{
>> +     return 0;
>> +}
>> +
>> +static u64 psp_v11_0_xgmi_get_hive_id(struct psp_context *psp)
>> +{
>> +     u64 hive_id = 0;
>> +
>> +     /* Remove me when normal psp interface is ready */
>> +     if (psp->adev->gmc.xgmi.num_physical_nodes)
>> +             hive_id = 0x123456789abcdef;
>> +
>> +     return hive_id;
>> +}
>> +
>>    static const struct psp_funcs psp_v11_0_funcs = {
>>        .init_microcode = psp_v11_0_init_microcode,
>>        .bootloader_load_sysdrv = psp_v11_0_bootloader_load_sysdrv,
>> @@ -560,6 +583,9 @@ static const struct psp_funcs psp_v11_0_funcs = {
>>        .cmd_submit = psp_v11_0_cmd_submit,
>>        .compare_sram_data = psp_v11_0_compare_sram_data,
>>        .mode1_reset = psp_v11_0_mode1_reset,
>> +     .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
>> +     .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
>> +     .xgmi_get_hive_id = psp_v11_0_xgmi_get_hive_id,
>>    };
>>
>>    void psp_v11_0_set_psp_funcs(struct psp_context *psp)
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx