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