>-----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. > >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
