[PATCH 1/5] drm/amdgpu: Don't reallocate ucode bo when suspend

Mon Oct 8 18:21:51 UTC 2018

Am 08.10.2018 um 20:15 schrieb Zhu, Rex:
>
>> -----Original Message-----
>> From: Koenig, Christian
>> Sent: Tuesday, October 9, 2018 2:03 AM
>> To: Zhu, Rex <Rex.Zhu at amd.com>; Deucher, Alexander
>> <Alexander.Deucher at amd.com>; Alex Deucher <alexdeucher at gmail.com>
>> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
>> Subject: Re: [PATCH 1/5] drm/amdgpu: Don't reallocate ucode bo when
>> suspend
>>
>> Am 08.10.2018 um 19:58 schrieb Zhu, Rex:
>>>> -----Original Message-----
>>>> From: Christian König <ckoenig.leichtzumerken at gmail.com>
>>>> Sent: Tuesday, October 9, 2018 1:32 AM
>>>> To: Zhu, Rex <Rex.Zhu at amd.com>; Deucher, Alexander
>>>> <Alexander.Deucher at amd.com>; Alex Deucher <alexdeucher at gmail.com>
>>>> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
>>>> Subject: Re: [PATCH 1/5] drm/amdgpu: Don't reallocate ucode bo when
>>>> suspend
>>>>
>>>> Am 08.10.2018 um 18:30 schrieb Zhu, Rex:
>>>>>> -----Original Message-----
>>>>>> From: Deucher, Alexander
>>>>>> Sent: Tuesday, October 9, 2018 12:21 AM
>>>>>> To: Zhu, Rex <Rex.Zhu at amd.com>; Alex Deucher
>>>> <alexdeucher at gmail.com>
>>>>>> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
>>>>>> Subject: RE: [PATCH 1/5] drm/amdgpu: Don't reallocate ucode bo when
>>>>>> suspend
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf
>> Of
>>>>>>> Zhu, Rex
>>>>>>> Sent: Monday, October 8, 2018 11:57 AM
>>>>>>> To: Alex Deucher <alexdeucher at gmail.com>
>>>>>>> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
>>>>>>> Subject: RE: [PATCH 1/5] drm/amdgpu: Don't reallocate ucode bo
>>>>>>> when suspend
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Alex Deucher <alexdeucher at gmail.com>
>>>>>>>> Sent: Thursday, October 4, 2018 11:35 AM
>>>>>>>> To: Zhu, Rex <Rex.Zhu at amd.com>
>>>>>>>> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
>>>>>>>> Subject: Re: [PATCH 1/5] drm/amdgpu: Don't reallocate ucode bo
>>>>>>>> when suspend
>>>>>>>>
>>>>>>>> On Wed, Oct 3, 2018 at 7:11 AM Rex Zhu <Rex.Zhu at amd.com> wrote:
>>>>>>>>> driver don't release the ucode memory when suspend. so don't
>>>>>>>>> need to allocate bo when resume back.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>
>>>>>>>>> ---
>>>>>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 2 +-
>>>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
>>>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
>>>>>>>>> index 9878212..adfeb93 100644
>>>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
>>>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
>>>>>>>>> @@ -434,7 +434,7 @@ int amdgpu_ucode_init_bo(struct
>>>>>> amdgpu_device
>>>>>>>> *adev)
>>>>>>>>>                    return 0;
>>>>>>>>>            }
>>>>>>>>>
>>>>>>>>> -       if (!adev->in_gpu_reset) {
>>>>>>>>> +       if (!adev->in_gpu_reset && !adev->in_suspend) {
>>>>>>>>>                    err = amdgpu_bo_create_kernel(adev,
>>>>>>>>> adev->firmware.fw_size,
>>>>>>>> PAGE_SIZE,
>>>>>>>>>                                            amdgpu_sriov_vf(adev) ?
>>>>>>>> AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
>>>>>>>>> &adev->firmware.fw_buf,
>>>>>>>> Not sure if we support S3 in SR-IOV, but I think this will break
>>>>>>>> it because we'll lose vram contents and not re-init it.
>>>>>>> Confirm with SR-IOV team, S3 was not supported in SR-IOV.
>>>>>>>
>>>>>>>     But I still confused why this patch will break the suspend if
>>>>>>> in SRIOV
>>>> case?
>>>>>> Pinned buffers don't get evicted so if we lose VRAM due to a gpu
>>>>>> reset or S3, the data is lost.  GTT is retained since the OS manages that.
>>>>> The gart table was unpinned when suspend.so don't need to create the
>>>>> bo
>>>> again. we still copy the ucode to the bo.
>>>>> And in baremetal,  this function can return directly for S3.
>>>> That's irrelevant.
>>>>
>>>> The whole code is buggy since amdgpu_ucode_fini_bo() will drop the BO
>>>> independent if we are in reset or in suspend.
>>> We don't call amdgpu_ucode_fini_bo when suspend/sriov_reset.
>> Yeah and exactly that's the bug which should be fixed instead.
> Sorry, I don't understand.
> Why we need to release the bo when suspend?
> In amdgpu, we create a bunch of bo(vram_scratch.robj, e.g), we only free them when driver unload.

You should not release the BO while suspending, but the 
amdgpu_ucode_fini_bo() and amdgpu_ucode_init_bo() should be called under 
the same conditions.

This means we should either separate filling the BO from allocating it 
(e.g. split amdgpu_ucode_init_bo into two functions) and then only call 
the filling function during GPU reset and resume.

Or we add the same "if (!adev->in_gpu_reset && !adev->in_suspend)" into 
the amdgpu_ucode_fini_bo() function as well.

I consider the first one more cleaner.

Christian.

>
> Rex
>
>> Christian.
>>
>>> Rex
>>>
>>>> The correct handling here is to remove the if all together and make
>>>> sure
>>>> amdgpu_bo_create_kernel() is ALWAYS called.
>>>>
>>>> Cause then it is always re-created if it isn't there already.
>>>>
>>>> Alternatively we could fix up the callers of amdgpu_ucode_init_bo()
>>>> and
>>>> amdgpu_ucode_fini_bo() to be correctly balanced.
>>>>
>>>> Christian.
>>>>
>>>>> Rex
>>>>>
>>>>>
>>>>>> Alex
>>>>>>
>>>>>>> Rex
>>>>>>>
>>>>>>>> Alex
>>>>>>>>
>>>>>>>>> --
>>>>>>>>> 1.9.1
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> amd-gfx mailing list
>>>>>>>>> amd-gfx at lists.freedesktop.org
>>>>>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>>>> _______________________________________________
>>>>>>> amd-gfx mailing list
>>>>>>> amd-gfx at lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>> _______________________________________________
>>>>> amd-gfx mailing list
>>>>> amd-gfx at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx