[PATCH] drm/amdgpu: Don't need to call csb_vram_unpin

[Deng, Emily]

>-----Original Message-----
>From: Koenig, Christian <Christian.Koenig at amd.com>
>Sent: Tuesday, May 28, 2019 3:43 PM
>To: Deng, Emily <Emily.Deng at amd.com>; Quan, Evan
><Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
>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 at amd.com>
>>> Sent: Tuesday, May 28, 2019 3:04 PM
>>> To: Quan, Evan <Evan.Quan at amd.com>; Deng, Emily
><Emily.Deng at amd.com>;
>>> amd-gfx at lists.freedesktop.org
>>> 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 at lists.freedesktop.org> On Behalf Of
>>>>> Christian K?nig
>>>>> Sent: Monday, May 27, 2019 7:02 PM
>>>>> To: Deng, Emily <Emily.Deng at amd.com>; amd-
>gfx at lists.freedesktop.org
>>>>> 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 at amd.com>
>>>>>> ---
>>>>>>     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 at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx