Am 07.03.19 um 17:59 schrieb 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@xxxxxxx>
>>> ---
>>> 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.
IIRC we already freed up a couple of other BOs at this place, e.g. the
ucode etc..
And you moved the ip_pool_init directly before the ucode allocation, so
that seems to be a good choice for freeing it up again.
Christian.
>
> 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);
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
