AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Koenig, Christian" <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>
To: "Quan, Evan" <Evan.Quan-5C7GfCeVMHo@public.gmane.org>,
	"Deng, Emily" <Emily.Deng-5C7GfCeVMHo@public.gmane.org>,
	"amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org"
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH] drm/amdgpu: Don't need to call csb_vram_unpin
Date: Tue, 28 May 2019 07:03:36 +0000	[thread overview]
Message-ID: <82dfcb34-109c-b7d6-c511-404008589869@amd.com> (raw)
In-Reply-To: <MN2PR12MB3344714A52B709FCE058F48DE41E0-rweVpJHSKToDMgCC8P//OwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>

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

  parent reply	other threads:[~2019-05-28  7:03 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-27  8:41 [PATCH] drm/amdgpu: Don't need to call csb_vram_unpin Emily Deng
     [not found] ` <1558946487-18034-1-git-send-email-Emily.Deng-5C7GfCeVMHo@public.gmane.org>
2019-05-27  9:31   ` Quan, Evan
2019-05-27 11:02   ` Christian König
     [not found]     ` <e15ac75c-fb1a-2a05-b857-d3761f5c905e-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-05-28  1:42       ` Quan, Evan
     [not found]         ` <MN2PR12MB3344714A52B709FCE058F48DE41E0-rweVpJHSKToDMgCC8P//OwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-05-28  7:03           ` Koenig, Christian [this message]
     [not found]             ` <82dfcb34-109c-b7d6-c511-404008589869-5C7GfCeVMHo@public.gmane.org>
2019-05-28  7:38               ` Deng, Emily
     [not found]                 ` <MWHPR12MB132646027A5B99282AB6557A8F1E0-Gy0DoCVfaSVaj5rVPFIlogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-05-28  7:43                   ` Koenig, Christian
     [not found]                     ` <daf490f4-49fd-3688-84ff-5dc47eaa535c-5C7GfCeVMHo@public.gmane.org>
2019-05-28  7:48                       ` Deng, Emily
  -- strict thread matches above, loose matches on Subject: below --
2019-05-24 10:33 Emily Deng
     [not found] ` <1558694002-19915-1-git-send-email-Emily.Deng-5C7GfCeVMHo@public.gmane.org>
2019-05-27  3:08   ` Deng, Emily
     [not found]     ` <MWHPR12MB1326D8B853B29C387FC1874B8F1D0-Gy0DoCVfaSVaj5rVPFIlogdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-05-27  6:26       ` Quan, Evan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=82dfcb34-109c-b7d6-c511-404008589869@amd.com \
    --to=christian.koenig-5c7gfcevmho@public.gmane.org \
    --cc=Emily.Deng-5C7GfCeVMHo@public.gmane.org \
    --cc=Evan.Quan-5C7GfCeVMHo@public.gmane.org \
    --cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox