[AMD Official Use Only - Internal Distribution Only] Hi Nirmoy, I also found amdgpu_device_ip_reinit_late_sriov() part is missed. Will push another patch to make them consistent soon. Best regards, Jiawei -----Original Message----- From: Das, Nirmoy <Nirmoy.Das@xxxxxxx> Sent: Friday, August 28, 2020 2:33 PM To: Deng, Emily <Emily.Deng@xxxxxxx>; Das, Nirmoy <Nirmoy.Das@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Liu, Monk <Monk.Liu@xxxxxxx>; Gu, JiaWei (Will) <JiaWei.Gu@xxxxxxx> Subject: Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov On 8/28/20 3:16 AM, Deng, Emily wrote: > [AMD Official Use Only - Internal Distribution Only] > > Hi Nirmoy, > Still think the original logical is more clear. No problem but we should at least make sure amdgpu_device_ip_reinit_late_sriov() and amdgpu_device_ip_reinit_early_sriov() are consistent. The last patch from Jiawei only touched amdgpu_device_ip_reinit_early_sriov(), same optimization should apply to amdgpu_device_ip_reinit_late_sriov() Regards, Nirmoy > > Best wishes > Emily Deng > > > >> -----Original Message----- >> From: Das, Nirmoy <Nirmoy.Das@xxxxxxx> >> Sent: Thursday, August 27, 2020 11:19 PM >> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx >> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Liu, Monk >> <Monk.Liu@xxxxxxx>; Gu, JiaWei (Will) <JiaWei.Gu@xxxxxxx>; Deng, >> Emily <Emily.Deng@xxxxxxx>; Das, Nirmoy <Nirmoy.Das@xxxxxxx> >> Subject: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov >> >> This patch removes some unwanted code duplication and simplifies >> sriov's ip block reinit logic. >> >> Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxx> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 117 >> +++++++++++---------- >> 1 file changed, 60 insertions(+), 57 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index 696a61cc3ac6..0db6db03bcd3 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -2587,77 +2587,80 @@ int amdgpu_device_ip_suspend(struct >> amdgpu_device *adev) return r; } >> >> -static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device >> *adev) >> +/** amdgpu_device_is_early_ip_block_sriov - check for early >> +ip_blocks >> + * >> + * @ip_block: ip block that need to be check >> + * >> + * Returns a tri-state value for a given ip block. >> + * If an ip block requires early reinit sriov then return 1 or 0 otherwise. >> + * Return -1 on invalid ip block. >> + * >> + */ >> + >> +static int >> +amdgpu_device_is_early_ip_block_sriov(const enum amd_ip_block_type >> +ip_block) >> { >> -int i, r; >> +switch (ip_block) { >> +/* early ip blocks */ >> +case AMD_IP_BLOCK_TYPE_GMC: >> +case AMD_IP_BLOCK_TYPE_COMMON: >> +case AMD_IP_BLOCK_TYPE_PSP: >> +case AMD_IP_BLOCK_TYPE_IH: >> +return 1; >> +/* late ip blocks */ >> +case AMD_IP_BLOCK_TYPE_SMC: >> +case AMD_IP_BLOCK_TYPE_DCE: >> +case AMD_IP_BLOCK_TYPE_GFX: >> +case AMD_IP_BLOCK_TYPE_SDMA: >> +case AMD_IP_BLOCK_TYPE_UVD: >> +case AMD_IP_BLOCK_TYPE_VCE: >> +case AMD_IP_BLOCK_TYPE_VCN: >> +return 0; >> +/* invalid ip block */ >> +default: >> +return -1; >> +} >> +} >> >> -static enum amd_ip_block_type ip_order[] = { -AMD_IP_BLOCK_TYPE_GMC, >> -AMD_IP_BLOCK_TYPE_COMMON, -AMD_IP_BLOCK_TYPE_PSP, >> -AMD_IP_BLOCK_TYPE_IH, -}; >> +static int amdgpu_device_ip_reinit_sriov(struct amdgpu_device *adev, >> +const int is_early) { int i; >> >> for (i = 0; i < adev->num_ip_blocks; i++) { -int j; >> +int r = 0; >> +bool init_ip; >> struct amdgpu_ip_block *block; >> +enum amd_ip_block_type ip_block; >> >> block = &adev->ip_blocks[i]; >> block->status.hw = false; >> +ip_block = block->version->type; >> +init_ip = (is_early == >> + amdgpu_device_is_early_ip_block_sriov(ip_block)); >> >> -for (j = 0; j < ARRAY_SIZE(ip_order); j++) { >> - >> -if (block->version->type != ip_order[j] || >> -!block->status.valid) >> -continue; >> +if (!init_ip || >> + (!is_early && block->status.hw) || >> + !block->status.valid) >> +continue; >> >> -r = block->version->funcs->hw_init(adev); >> -DRM_INFO("RE-INIT-early: %s %s\n", block->version- >>> funcs->name, r?"failed":"succeeded"); >> -if (r) >> -return r; >> -block->status.hw = true; >> +if (init_ip && (ip_block == AMD_IP_BLOCK_TYPE_SMC)) { r = >> +block->version->funcs->resume(adev); >> +goto show_log; >> } >> -} >> - >> -return 0; >> -} >> >> -static int amdgpu_device_ip_reinit_late_sriov(struct amdgpu_device >> *adev) -{ -int i, r; >> - >> -static enum amd_ip_block_type ip_order[] = { -AMD_IP_BLOCK_TYPE_SMC, >> -AMD_IP_BLOCK_TYPE_DCE, -AMD_IP_BLOCK_TYPE_GFX, >> -AMD_IP_BLOCK_TYPE_SDMA, -AMD_IP_BLOCK_TYPE_UVD, >> -AMD_IP_BLOCK_TYPE_VCE, -AMD_IP_BLOCK_TYPE_VCN -}; >> - >> -for (i = 0; i < ARRAY_SIZE(ip_order); i++) { -int j; -struct >> amdgpu_ip_block *block; >> +if (init_ip) >> +r = block->version->funcs->hw_init(adev); >> >> -for (j = 0; j < adev->num_ip_blocks; j++) { -block = >> &adev->ip_blocks[j]; >> +show_log: >> +DRM_INFO("RE-INIT-%s: %s %s\n", is_early ? "early":"late", >> + block->version->funcs->name, r ? >> "failed":"succeeded"); >> >> -if (block->version->type != ip_order[i] || -!block->status.valid || >> -block->status.hw) >> -continue; >> +if (r) >> +return r; >> >> -if (block->version->type == >> AMD_IP_BLOCK_TYPE_SMC) >> -r = block->version->funcs->resume(adev); >> -else >> -r = block->version->funcs->hw_init(adev); >> +block->status.hw = true; >> >> -DRM_INFO("RE-INIT-late: %s %s\n", block->version- >>> funcs->name, r?"failed":"succeeded"); >> -if (r) >> -return r; >> -block->status.hw = true; >> -} >> } >> >> return 0; >> @@ -3901,7 +3904,7 @@ static int amdgpu_device_reset_sriov(struct >> amdgpu_device *adev, amdgpu_amdkfd_pre_reset(adev); >> >> /* Resume IP prior to SMC */ >> -r = amdgpu_device_ip_reinit_early_sriov(adev); >> +r = amdgpu_device_ip_reinit_sriov(adev, 1); >> if (r) >> goto error; >> >> @@ -3914,7 +3917,7 @@ static int amdgpu_device_reset_sriov(struct >> amdgpu_device *adev, return r; >> >> /* now we are okay to resume SMC/CP/SDMA */ -r = >> amdgpu_device_ip_reinit_late_sriov(adev); >> +r = amdgpu_device_ip_reinit_sriov(adev, 0); >> if (r) >> goto error; >> >> -- >> 2.28.0 _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx