[PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

[Gu, JiaWei (Will)]

[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 at amd.com> 
Sent: Friday, August 28, 2020 2:33 PM
To: Deng, Emily <Emily.Deng at amd.com>; Das, Nirmoy <Nirmoy.Das at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Liu, Monk <Monk.Liu at amd.com>; Gu, JiaWei (Will) <JiaWei.Gu at amd.com>
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 at amd.com>
>> Sent: Thursday, August 27, 2020 11:19 PM
>> To: amd-gfx at lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Liu, Monk 
>> <Monk.Liu at amd.com>; Gu, JiaWei (Will) <JiaWei.Gu at amd.com>; Deng, 
>> Emily <Emily.Deng at amd.com>; Das, Nirmoy <Nirmoy.Das at amd.com>
>> 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 at amd.com>
>> ---
>> 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