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@xxxxxxx>; Deucher, Alexander
<Alexander.Deucher@xxxxxxx>; Alex Deucher <alexdeucher@xxxxxxxxx>
Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
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@xxxxxxxxx>
Sent: Tuesday, October 9, 2018 1:32 AM
To: Zhu, Rex <Rex.Zhu@xxxxxxx>; Deucher, Alexander
<Alexander.Deucher@xxxxxxx>; Alex Deucher <alexdeucher@xxxxxxxxx>
Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
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@xxxxxxx>; Alex Deucher
<alexdeucher@xxxxxxxxx>
Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Subject: RE: [PATCH 1/5] drm/amdgpu: Don't reallocate ucode bo when
suspend
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf
Of
Zhu, Rex
Sent: Monday, October 8, 2018 11:57 AM
To: Alex Deucher <alexdeucher@xxxxxxxxx>
Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Subject: RE: [PATCH 1/5] drm/amdgpu: Don't reallocate ucode bo
when suspend
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Thursday, October 4, 2018 11:35 AM
To: Zhu, Rex <Rex.Zhu@xxxxxxx>
Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
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@xxxxxxx> 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@xxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
