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

["Christian König" <christian.koenig-5C7GfCeVMHo@public.gmane.org>]

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@lists.freedesktop.org> on behalf of
>>> Christian König <ckoenig.leichtzumerken@gmail.com>
>>> Sent: Thursday, September 6, 2018 3:42:33 AM
>>> To: Liu, Shaoyun; amd-gfx@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@amd.com>
>>>>
>>>> Add dummy function for xgmi function interface with psp
>>>>
>>>> Change-Id: I01f35baf5a4b96e9654d448c9892be3cd72c05b7
>>>> Signed-off-by: Shaoyun Liu <Shaoyun.Liu@amd.com>
>>>> Reviewed-by: Felix Kuehling <Felix.Kuehling@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@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx