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

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

[Emily Deng]

As it will destory 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.

Signed-off-by: Emily Deng <Emily.Deng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 ----------------
 1 file changed, 16 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..231b9e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1154,20 +1154,6 @@ static int gfx_v9_0_csb_vram_pin(struct amdgpu_device *adev)
 	return r;
 }
 
-static void gfx_v9_0_csb_vram_unpin(struct amdgpu_device *adev)
-{
-	int r;
-
-	if (!adev->gfx.rlc.clear_state_obj)
-		return;
-
-	r = amdgpu_bo_reserve(adev->gfx.rlc.clear_state_obj, true);
-	if (likely(r == 0)) {
-		amdgpu_bo_unpin(adev->gfx.rlc.clear_state_obj);
-		amdgpu_bo_unreserve(adev->gfx.rlc.clear_state_obj);
-	}
-}
-
 static void gfx_v9_0_mec_fini(struct amdgpu_device *adev)
 {
 	amdgpu_bo_free_kernel(&adev->gfx.mec.hpd_eop_obj, NULL, NULL);
@@ -3385,8 +3371,6 @@ 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);
-
 	return 0;
 }
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

Ping......

Best wishes
Emily Deng
>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Emily
>Deng
>Sent: Friday, May 24, 2019 6:33 PM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deng, Emily <Emily.Deng@amd.com>
>Subject: [PATCH] drm/amdgpu: Don't need to call csb_vram_unpin
>
>[CAUTION: External Email]
>
>As it will destory 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.
>
>Signed-off-by: Emily Deng <Emily.Deng@amd.com>
>---
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 ----------------
> 1 file changed, 16 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..231b9e0 100644
>--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>@@ -1154,20 +1154,6 @@ static int gfx_v9_0_csb_vram_pin(struct
>amdgpu_device *adev)
>        return r;
> }
>
>-static void gfx_v9_0_csb_vram_unpin(struct amdgpu_device *adev) -{
>-       int r;
>-
>-       if (!adev->gfx.rlc.clear_state_obj)
>-               return;
>-
>-       r = amdgpu_bo_reserve(adev->gfx.rlc.clear_state_obj, true);
>-       if (likely(r == 0)) {
>-               amdgpu_bo_unpin(adev->gfx.rlc.clear_state_obj);
>-               amdgpu_bo_unreserve(adev->gfx.rlc.clear_state_obj);
>-       }
>-}
>-
> static void gfx_v9_0_mec_fini(struct amdgpu_device *adev)  {
>        amdgpu_bo_free_kernel(&adev->gfx.mec.hpd_eop_obj, NULL, NULL);
>@@ -3385,8 +3371,6 @@ 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);
>-
>        return 0;
> }
>
>--
>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

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

[Quan, Evan]

Did you ever test S3 suspend/resume? On S3 suspend/resume path, only hw_fini was called.

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Deng, Emily
> Sent: Monday, May 27, 2019 11:09 AM
> To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: RE: [PATCH] drm/amdgpu: Don't need to call csb_vram_unpin
> 
> Ping......
> 
> Best wishes
> Emily Deng
> >-----Original Message-----
> >From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> >Emily Deng
> >Sent: Friday, May 24, 2019 6:33 PM
> >To: amd-gfx@lists.freedesktop.org
> >Cc: Deng, Emily <Emily.Deng@amd.com>
> >Subject: [PATCH] drm/amdgpu: Don't need to call csb_vram_unpin
> >
> >[CAUTION: External Email]
> >
> >As it will destory 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.
> >
> >Signed-off-by: Emily Deng <Emily.Deng@amd.com>
> >---
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 ----------------
> > 1 file changed, 16 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..231b9e0 100644
> >--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> >+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> >@@ -1154,20 +1154,6 @@ static int gfx_v9_0_csb_vram_pin(struct
> >amdgpu_device *adev)
> >        return r;
> > }
> >
> >-static void gfx_v9_0_csb_vram_unpin(struct amdgpu_device *adev) -{
> >-       int r;
> >-
> >-       if (!adev->gfx.rlc.clear_state_obj)
> >-               return;
> >-
> >-       r = amdgpu_bo_reserve(adev->gfx.rlc.clear_state_obj, true);
> >-       if (likely(r == 0)) {
> >-               amdgpu_bo_unpin(adev->gfx.rlc.clear_state_obj);
> >-               amdgpu_bo_unreserve(adev->gfx.rlc.clear_state_obj);
> >-       }
> >-}
> >-
> > static void gfx_v9_0_mec_fini(struct amdgpu_device *adev)  {
> >        amdgpu_bo_free_kernel(&adev->gfx.mec.hpd_eop_obj, NULL, NULL);
> >@@ -3385,8 +3371,6 @@ 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);
> >-
> >        return 0;
> > }
> >
> >--
> >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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Deng, Emily]

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

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

[Koenig, Christian]

Am 28.05.19 um 09:38 schrieb Deng, Emily:
>> -----Original Message-----
>> From: Koenig, Christian <Christian.Koenig@amd.com>
>> Sent: Tuesday, May 28, 2019 3:04 PM
>> To: Quan, Evan <Evan.Quan@amd.com>; Deng, Emily
>> <Emily.Deng@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.

>> 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@lists.freedesktop.org> On Behalf Of
>>>> Christian K?nig
>>>> Sent: Monday, May 27, 2019 7:02 PM
>>>> To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@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@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

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

[Deng, Emily]

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

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

[Koenig, Christian]

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.

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@lists.freedesktop.org> On Behalf Of
>> Christian K?nig
>> Sent: Monday, May 27, 2019 7:02 PM
>> To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@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@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

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

[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@lists.freedesktop.org> On Behalf Of
> Christian K?nig
> Sent: Monday, May 27, 2019 7:02 PM
> To: Deng, Emily <Emily.Deng@amd.com>; amd-gfx@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@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

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

[Christian König]

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

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

[Quan, Evan]

Reviewed-by: Evan Quan <evan.quan@amd.com>

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Emily Deng
> Sent: Monday, May 27, 2019 4:41 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deng, Emily <Emily.Deng@amd.com>
> Subject: [PATCH] drm/amdgpu: Don't need to call csb_vram_unpin
> 
> 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@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);
> 
>  	return 0;
>  }
> --
> 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

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

[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@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);
 
 	return 0;
 }
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx