[PATCH] drm/amdgpu: Move IB pool init and fini.

[Grodzovsky, Andrey]

On 3/7/19 6:29 AM, Christian König wrote:
> Am 06.03.19 um 22:25 schrieb Andrey Grodzovsky:
>> Problem:
>> Using SDMA for TLB invalidation in certain ASICs exposed a problem
>> of IB pool not being ready while SDMA already up on Init and already
>> shutt down while SDMA still running on Fini. This caused
>> IB allocation failure. Temproary fix was commited into a
>> bringup branch but this is the generic fix.
>>
>> Fix:
>> Init IB pool rigth after GMC is ready but before SDMA is ready.
>> Do th opposite for Fini.
>>
>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 +++++++++++---------
>>   1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 00def57..c05a551 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1686,6 +1686,13 @@ static int amdgpu_device_ip_init(struct 
>> amdgpu_device *adev)
>>           }
>>       }
>>   +    r = amdgpu_ib_pool_init(adev);
>> +    if (r) {
>> +        dev_err(adev->dev, "IB initialization failed (%d).\n", r);
>> +        amdgpu_vf_error_put(adev, AMDGIM_ERROR_VF_IB_INIT_FAIL, 0, r);
>> +        goto init_failed;
>> +    }
>> +
>>       r = amdgpu_ucode_create_bo(adev); /* create ucode bo when 
>> sw_init complete*/
>>       if (r)
>>           goto init_failed;
>> @@ -1888,7 +1895,8 @@ static int amdgpu_device_ip_fini(struct 
>> amdgpu_device *adev)
>>       for (i = 0; i < adev->num_ip_blocks; i++) {
>>           if (!adev->ip_blocks[i].status.hw)
>>               continue;
>> -        if (adev->ip_blocks[i].version->type == 
>> AMD_IP_BLOCK_TYPE_SMC) {
>> +        if (adev->ip_blocks[i].version->type == 
>> AMD_IP_BLOCK_TYPE_SMC ||
>> +            adev->ip_blocks[i].version->type == 
>> AMD_IP_BLOCK_TYPE_SDMA) {
>
> Why is that necessary?


I assumed SDMA gw fini will call tlb flush but after testing with this 
removed it didn't so ok.

>
>>               r = adev->ip_blocks[i].version->funcs->hw_fini((void 
>> *)adev);
>>               /* XXX handle errors */
>>               if (r) {
>> @@ -1900,6 +1908,8 @@ static int amdgpu_device_ip_fini(struct 
>> amdgpu_device *adev)
>>           }
>>       }
>>   +    amdgpu_ib_pool_fini(adev);
>
> Thinking more about it this should probably be done right before GMC 
> SW fini.
>
> IIRC we already have an extra case for this somewhere.
>
> Christian.


Why do you think that the proper place ? Seemed to work fine where it was.

Anyway, i moved it and resent v2.

Andrey


>
>> +
>>       for (i = adev->num_ip_blocks - 1; i >= 0; i--) {
>>           if (!adev->ip_blocks[i].status.hw)
>>               continue;
>> @@ -2651,13 +2661,6 @@ int amdgpu_device_init(struct amdgpu_device 
>> *adev,
>>       /* Get a log2 for easy divisions. */
>>       adev->mm_stats.log2_max_MBps = ilog2(max(1u, max_MBps));
>>   -    r = amdgpu_ib_pool_init(adev);
>> -    if (r) {
>> -        dev_err(adev->dev, "IB initialization failed (%d).\n", r);
>> -        amdgpu_vf_error_put(adev, AMDGIM_ERROR_VF_IB_INIT_FAIL, 0, r);
>> -        goto failed;
>> -    }
>> -
>>       amdgpu_fbdev_init(adev);
>>         r = amdgpu_pm_sysfs_init(adev);
>> @@ -2735,7 +2738,6 @@ void amdgpu_device_fini(struct amdgpu_device 
>> *adev)
>>           else
>>               drm_atomic_helper_shutdown(adev->ddev);
>>       }
>> -    amdgpu_ib_pool_fini(adev);
>>       amdgpu_fence_driver_fini(adev);
>>       amdgpu_pm_sysfs_fini(adev);
>>       amdgpu_fbdev_fini(adev);
>