It's correct and already working  on vega10/tonga for days, 
In fact the guilty context already works at my side 

BR Monk



-----Original Message-----
From: Christian König [mailto:[email protected]] 
Sent: Wednesday, May 03, 2017 5:02 PM
To: Liu, Monk <[email protected]>; [email protected]
Subject: Re: [PATCH 3/6] drm/amdgpu:re-write sriov_reinit_early/late

Am 03.05.2017 um 05:48 schrieb Monk Liu:
> 1,this way we make those routines compatible with the sequence
>    requirment for both Tonga and Vega10 2,ignore PSP hw init when 
> doing TDR, because for SR-IOV device the ucode won't get lost after VF 
> FLR, so no need to invoke PSP doing the ucode reloading again.
>
> Signed-off-by: Monk Liu <[email protected]>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 63 
> ++++++++++++++++++------------
>   1 file changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 5161c20..5573792 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1718,19 +1718,27 @@ static int amdgpu_sriov_reinit_early(struct 
> amdgpu_device *adev)
>   {
>       int i, r;
>   
> -     for (i = 0; i < adev->num_ip_blocks; i++) {
> -             if (!adev->ip_blocks[i].status.valid)
> -                     continue;
> -
> -             if (adev->ip_blocks[i].version->type == 
> AMD_IP_BLOCK_TYPE_COMMON ||
> -                             adev->ip_blocks[i].version->type == 
> AMD_IP_BLOCK_TYPE_GMC ||
> -                             adev->ip_blocks[i].version->type == 
> AMD_IP_BLOCK_TYPE_IH)
> -                     r = adev->ip_blocks[i].version->funcs->hw_init(adev);
> +     static enum amd_ip_block_type ip_order[] = {
> +             AMD_IP_BLOCK_TYPE_GMC,
> +             AMD_IP_BLOCK_TYPE_COMMON,
> +             AMD_IP_BLOCK_TYPE_GFXHUB,
> +             AMD_IP_BLOCK_TYPE_MMHUB,
> +             AMD_IP_BLOCK_TYPE_IH,
> +     };
> +
> +     for (i = 0; i < sizeof(ip_order)/sizeof(ip_order[0]); i++) {

You should use ARRAY_SIZE here instead.

> +             int j;
> +             struct amdgpu_ip_block *block;
> +
> +             for (j = 0; j < adev->num_ip_blocks; j++) {
> +                     block = &adev->ip_blocks[j];
> +
> +                     if (block->version->type != ip_order[i] ||
> +                             !block->status.valid)
> +                             continue;
>   
> -             if (r) {
> -                     DRM_ERROR("resume of IP block <%s> failed %d\n",
> -                               adev->ip_blocks[i].version->funcs->name, r);
> -                     return r;
> +                     r = block->version->funcs->hw_init(adev);
> +                     DRM_INFO("RE-INIT: %s %s\n", 
> block->version->funcs->name, 
> +r?"failed":"successed");
>               }
>       }
>   
> @@ -1741,20 +1749,27 @@ static int amdgpu_sriov_reinit_late(struct 
> amdgpu_device *adev)
>   {
>       int i, r;
>   
> -     for (i = 0; i < adev->num_ip_blocks; i++) {
> -             if (!adev->ip_blocks[i].status.valid)
> -                     continue;
> +     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_VCE,
> +     };
>   
> -             if (adev->ip_blocks[i].version->type == 
> AMD_IP_BLOCK_TYPE_COMMON ||
> -                             adev->ip_blocks[i].version->type == 
> AMD_IP_BLOCK_TYPE_GMC ||
> -                             adev->ip_blocks[i].version->type == 
> AMD_IP_BLOCK_TYPE_IH )
> -                     continue;
> +     for (i = 0; i < sizeof(ip_order)/sizeof(ip_order[0]); i++) {

And here as well.

> +             int j;
> +             struct amdgpu_ip_block *block;
>   
> -             r = adev->ip_blocks[i].version->funcs->hw_init(adev);
> -             if (r) {
> -                     DRM_ERROR("resume of IP block <%s> failed %d\n",
> -                               adev->ip_blocks[i].version->funcs->name, r);
> -                     return r;
> +             for (j = 0; j < adev->num_ip_blocks; j++) {
> +                     block = &adev->ip_blocks[j];
> +
> +                     if (block->version->type != ip_order[i] ||
> +                             !block->status.valid)
> +                             continue;
> +
> +                     r = block->version->funcs->hw_init(adev);
> +                     DRM_INFO("RE-INIT: %s %s\n", 
> block->version->funcs->name, 
> +r?"failed":"successed");

This changes the order in which blocks are initialized which is probably not 
correct.

Alex needs to take a look at this, but we clearly need to improve the handling 
here.

Regards,
Christian.

>               }
>       }
>   


_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to