>-----Original Message----- >From: Koenig, Christian <Christian.Koenig@xxxxxxx> >Sent: Tuesday, May 28, 2019 3:43 PM >To: Deng, Emily <Emily.Deng@xxxxxxx>; Quan, Evan ><Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx >Subject: Re: [PATCH] drm/amdgpu: Don't need to call csb_vram_unpin > >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. Thanks for your good suggestion. Will revert this patch, and submit another patch. Best wishes Emily Deng > >>> 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
