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

[Gu, JiaWei (Will)]

[AMD Official Use Only - Internal Distribution Only]

Per Monk and Emily's suggestion, I will not submit another patch to make amdgpu_device_ip_reinit_late_sriov()
and amdgpu_device_ip_reinit_early_sriov() consistent.
There's already no logic bug, the little difference in loop layer order does no harm.

Sorry about missing the amdgpu_device_ip_reinit_late_sriov() part.

Best regards,
Jiawei

-----Original Message-----
From: Das, Nirmoy <Nirmoy.Das at amd.com> 
Sent: Friday, August 28, 2020 3:14 PM
To: Gu, JiaWei (Will) <JiaWei.Gu at amd.com>; Das, Nirmoy <Nirmoy.Das at amd.com>; Deng, Emily <Emily.Deng at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Liu, Monk <Monk.Liu at amd.com>
Subject: Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov


On 8/28/20 8:58 AM, Gu, JiaWei (Will) wrote:
> [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.


Thanks, Jiawei.


Nirmoy


>
> 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