Hi Christian,
I have reverted the before change as your suggestion, and sent this new
patch, could you help to review this?
Best wishes
Emily Deng
>-----Original Message-----
>From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> On Behalf Of Deng,
>Emily
>Sent: Wednesday, May 29, 2019 10:52 AM
>To: amd-gfx@lists.freedesktop.org
>Subject: RE: [PATCH] drm/amdgpu:Fix the unpin warning about csb buffer
>
>Ping......
>
>Best wishes
>Emily Deng
>
>
>
>>-----Original Message-----
>>From: Deng, Emily <emily.d...@amd.com>
>>Sent: Tuesday, May 28, 2019 6:14 PM
>>To: Deng, Emily <emily.d...@amd.com>; amd-gfx@lists.freedesktop.org
>>Subject: RE: [PATCH] drm/amdgpu:Fix the unpin warning about csb buffer
>>
>>Ping ......
>>
>>Best wishes
>>Emily Deng
>>
>>
>>
>>>-----Original Message-----
>>>From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> On Behalf Of
>>>Emily Deng
>>>Sent: Tuesday, May 28, 2019 4:06 PM
>>>To: amd-gfx@lists.freedesktop.org
>>>Cc: Deng, Emily <emily.d...@amd.com>
>>>Subject: [PATCH] drm/amdgpu:Fix the unpin warning about csb buffer
>>>
>>>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 amdgpu_bo_free_kernel in
>>>gfx_v9_0_sw_fini, or it will have unpin warning.
>>>
>>>Signed-off-by: Emily Deng <emily.d...@amd.com>
>>>---
>>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 4 +---
>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>>b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>>index c763733..cc5a382 100644
>>>--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>>+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>>@@ -1794,9 +1794,7 @@ static int gfx_v9_0_sw_fini(void *handle)
>>>
>>> gfx_v9_0_mec_fini(adev);
>>> gfx_v9_0_ngg_fini(adev);
>>>- amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,
>>>- &adev->gfx.rlc.clear_state_gpu_addr,
>>>- (void **)&adev->gfx.rlc.cs_ptr);
>>>+ amdgpu_bo_unref(&adev->gfx.rlc.clear_state_obj);
>>> if (adev->asic_type == CHIP_RAVEN) {
>>> amdgpu_bo_free_kernel(&adev->gfx.rlc.cp_table_obj,
>>> &adev->gfx.rlc.cp_table_gpu_addr,
>>>--
>>>2.7.4
>>>
>>>_______________________________________________
>>>amd-gfx mailing list
>>>amd-gfx@lists.freedesktop.org
>>>https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>_______________________________________________
>amd-gfx mailing list
>amd-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/amd-gfx
--- Begin Message ---
>-----Original Message-----
>From: Koenig, Christian <christian.koe...@amd.com>
>Sent: Tuesday, May 28, 2019 3:43 PM
>To: Deng, Emily <emily.d...@amd.com>; Quan, Evan
><evan.q...@amd.com>; amd-gfx@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.koe...@amd.com>
>>> Sent: Tuesday, May 28, 2019 3:04 PM
>>> To: Quan, Evan <evan.q...@amd.com>; Deng, Emily
><emily.d...@amd.com>;
>>> amd-gfx@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-boun...@lists.freedesktop.org> On Behalf Of
>>>>> Christian K?nig
>>>>> Sent: Monday, May 27, 2019 7:02 PM
>>>>> To: Deng, Emily <emily.d...@amd.com>; amd-
>g...@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.d...@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@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
--- End Message ---
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
