Ok in this case feel free to add my Acked-by. Christian. Am 07.09.2018 um 20:10 schrieb Felix Kuehling: > On 2018-09-07 05:05 AM, Christian König wrote: >> Well, exactly that's my point. How certain are we that the PSP >> firmware interface is not going to change? > The interface has been defined and agreed on and is being implemented at > the moment. The placeholders here are based on that design. Of course > that's not a guarantee that it won't change, but it's the best we can do > to be prepared and allow us to make progress in the mean time. > >> At bare minimum we should add big "/* TODOD: .. */" markers all around >> explaining why those functions aren't implemented yet. > OK. > > Regards, >  Felix > >> 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