Am 28.05.19 um 09:38 schrieb Deng, Emily: >> -----Original Message----- >> From: Koenig, Christian <Christian.Koenig@xxxxxxx> >> Sent: Tuesday, May 28, 2019 3:04 PM >> To: Quan, Evan <Evan.Quan@xxxxxxx>; Deng, Emily >> <Emily.Deng@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx >> Subject: Re: [PATCH] drm/amdgpu: Don't need to call csb_vram_unpin >> >> Ok in this case the patch is a NAK. >> >> The correct solution is to stop using amdgpu_bo_free_kernel in >> gfx_v9_0_sw_fini. > So we just lead the memory leak here and not destroy the bo? I don't think it is correct. Oh, no. That's not what I meant. We should stop using amdgpu_bo_free_kernel and instead use amdgpu_bo_free! Sorry for not being clear here, Christian. >> BTW: Are we using the kernel pointer somewhere? Cause that one became >> completely invalid because of patch "drm/amdgpu: pin the csb buffer on hw >> init". >> >> Christian. >> >> Am 28.05.19 um 03:42 schrieb Quan, Evan: >>> The original unpin in hw_fini was introduced by >>> https://lists.freedesktop.org/archives/amd-gfx/2018-July/023681.html >>> >>> Evan >>>> -----Original Message----- >>>> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of >>>> Christian K?nig >>>> Sent: Monday, May 27, 2019 7:02 PM >>>> To: Deng, Emily <Emily.Deng@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx >>>> Subject: Re: [PATCH] drm/amdgpu: Don't need to call csb_vram_unpin >>>> >>>> Am 27.05.19 um 10:41 schrieb Emily Deng: >>>>> As it will destroy clear_state_obj, and also will unpin it in the >>>>> gfx_v9_0_sw_fini, so don't need to call csb_vram unpin in >>>>> gfx_v9_0_hw_fini, or it will have unpin warning. >>>>> >>>>> v2: For suspend, still need to do unpin >>>>> >>>>> Signed-off-by: Emily Deng <Emily.Deng@xxxxxxx> >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >>>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >>>>> index 5eb70e8..5b1ff48 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c >>>>> @@ -3395,7 +3395,8 @@ static int gfx_v9_0_hw_fini(void *handle) >>>>> gfx_v9_0_cp_enable(adev, false); >>>>> adev->gfx.rlc.funcs->stop(adev); >>>>> >>>>> - gfx_v9_0_csb_vram_unpin(adev); >>>>> + if (adev->in_suspend) >>>>> + gfx_v9_0_csb_vram_unpin(adev); >>>> That doesn't looks like a good idea to me. >>>> >>>> Why do we have unpin both in the sw_fini as well as the hw_fini code >> paths? >>>> Regards, >>>> Christian. >>>> >>>>> return 0; >>>>> } >>>> _______________________________________________ >>>> 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
