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

[Liu, Monk]

[AMD Official Use Only - Internal Distribution Only]

>> The patch was to fix the previous inconsistent optimization patch.

If so that's a good reason, I'll take a look , thanks


-----邮件原件-----
发件人: Das, Nirmoy <Nirmoy.Das at amd.com>
发送时间: 2020年8月28日 15:46
收件人: Liu, Monk <Monk.Liu at amd.com>; Das, Nirmoy <Nirmoy.Das at amd.com>; amd-gfx at lists.freedesktop.org
抄送: Deucher, Alexander <Alexander.Deucher at amd.com>; Gu, JiaWei (Will) <JiaWei.Gu at amd.com>; Deng, Emily <Emily.Deng at amd.com>
主题: Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov


On 8/28/20 9:29 AM, Liu, Monk wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Please don't change those code unless you have a full stress test and
> a solid reason (what bug fixed or what new feature introduced )
>
> Otherwise if it's a pure personal refactor or cleanup it will be not
> necessary


Hi Monk,


The patch was to fix the previous inconsistent optimization patch.

I don't have a sriov device so definitely wouldn't merge it without a tested-by tag so I added all relevant people.


Regards,

Nirmoy

>
> _____________________________________
> Monk Liu|GPU Virtualization Team |AMD
>
>
> -----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
>