RE: [PATCH 5/5] drm/amdgpu: fix calltrace during kmd unload

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



It is tested/verified by Xiaojie  on nv14 against S3 issues.

Thanks 

_____________________________________
Monk Liu|GPU Virtualization Team |AMD


-----Original Message-----
From: Koenig, Christian <Christian.Koenig@xxxxxxx> 
Sent: Friday, November 29, 2019 3:46 PM
To: Liu, Monk <Monk.Liu@xxxxxxx>; Yuan, Xiaojie <Xiaojie.Yuan@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 5/5] drm/amdgpu: fix calltrace during kmd unload

In this case feel free to add an Acked-by: Christian König <christian.koenig@xxxxxxx> to your patch.

But I would recommend to get an rb from somebody which knows that stuff better than I do.

Regards,
Christian.

Am 29.11.19 um 02:58 schrieb Liu, Monk:
> The content of CSIB is always static, I submitted a patch to use the 
> re-init and get rid of pin/unpin CSIB in hw_ini/fini,  (my purpose is 
> to fix the double unpin warning during unload ) 
> _____________________________________
> Monk Liu|GPU Virtualization Team |AMD
>
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
> Sent: Thursday, November 28, 2019 7:51 PM
> To: Liu, Monk <Monk.Liu@xxxxxxx>; Yuan, Xiaojie 
> <Xiaojie.Yuan@xxxxxxx>; Deucher, Alexander 
> <Alexander.Deucher@xxxxxxx>; Koenig, Christian 
> <Christian.Koenig@xxxxxxx>
> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 5/5] drm/amdgpu: fix calltrace during kmd unload
>
> Hi Monk,
>
> if the content of the CSIB is constant then it is certainly better to just re-initialize it.
>
> This also prevents from corruption because of VRAM lost.
>
> Christian.
>
> Am 28.11.19 um 03:53 schrieb Liu, Monk:
>> Hi Xiaojie
>>
>> For SRIOV we don't use suspend so I didn't think to that part, thanks for the remind !
>> But we still need to fix this call trace issue anyway (our jenkins 
>> testing  system consider such call trace as an error )
>>
>> How about we do "  adev->gfx.rlc.funcs->get_csb_buffer(adev,
>> dst_ptr);" in the hw_init() ? this way You don't need to evict the CSIB during suspend and the CSIB always will be re-initialized after S3 resume ?
>>
>> @Deucher, Alexander @Koenig, Christian what's your opinion ?
>> _____________________________________
>> Monk Liu|GPU Virtualization Team |AMD
>>
>>
>> -----Original Message-----
>> From: Yuan, Xiaojie <Xiaojie.Yuan@xxxxxxx>
>> Sent: Tuesday, November 26, 2019 9:10 PM
>> To: Liu, Monk <Monk.Liu@xxxxxxx>
>> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH 5/5] drm/amdgpu: fix calltrace during kmd unload
>>
>> Hi Monk,
>>
>> hw_fini() is called in suspend code path as well. I'm wondering how csb can be evicted if it's not unpined before suspend.
>>
>> BR,
>> Xiaojie
>>
>>> On Nov 26, 2019, at 7:50 PM, Monk Liu <Monk.Liu@xxxxxxx> wrote:
>>>
>>> kernel would report a warning on double unpin on the csb BO because 
>>> we unpin it during hw_fini but actually we don't need to pin/unpin 
>>> it during hw_init/fini since it is created with kernel pinned
>>>
>>> remove all those useless code for gfx9/10
>>>
>>> Signed-off-by: Monk Liu <Monk.Liu@xxxxxxx>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c |  1 - 
>>> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  | 38 --------------------------------
>>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 39 ---------------------------------
>>> 3 files changed, 78 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
>>> index c8793e6..289fada 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
>>> @@ -145,7 +145,6 @@ int amdgpu_gfx_rlc_init_csb(struct amdgpu_device *adev)
>>>      dst_ptr = adev->gfx.rlc.cs_ptr;
>>>      adev->gfx.rlc.funcs->get_csb_buffer(adev, dst_ptr);
>>>      amdgpu_bo_kunmap(adev->gfx.rlc.clear_state_obj);
>>> -    amdgpu_bo_unpin(adev->gfx.rlc.clear_state_obj);
>>>      amdgpu_bo_unreserve(adev->gfx.rlc.clear_state_obj);
>>>
>>>      return 0;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>> index a56cba9..5ee7467 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>> @@ -996,39 +996,6 @@ static int gfx_v10_0_rlc_init(struct amdgpu_device *adev)
>>>      return 0;
>>> }
>>>
>>> -static int gfx_v10_0_csb_vram_pin(struct amdgpu_device *adev) -{
>>> -    int r;
>>> -
>>> -    r = amdgpu_bo_reserve(adev->gfx.rlc.clear_state_obj, false);
>>> -    if (unlikely(r != 0))
>>> -        return r;
>>> -
>>> -    r = amdgpu_bo_pin(adev->gfx.rlc.clear_state_obj,
>>> -            AMDGPU_GEM_DOMAIN_VRAM);
>>> -    if (!r)
>>> -        adev->gfx.rlc.clear_state_gpu_addr =
>>> -            amdgpu_bo_gpu_offset(adev->gfx.rlc.clear_state_obj);
>>> -
>>> -    amdgpu_bo_unreserve(adev->gfx.rlc.clear_state_obj);
>>> -
>>> -    return r;
>>> -}
>>> -
>>> -static void gfx_v10_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_v10_0_mec_fini(struct amdgpu_device *adev) {
>>>      amdgpu_bo_free_kernel(&adev->gfx.mec.hpd_eop_obj, NULL, NULL); 
>>> @@
>>> -3780,10 +3747,6 @@ static int gfx_v10_0_hw_init(void *handle)
>>>      int r;
>>>      struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>>
>>> -    r = gfx_v10_0_csb_vram_pin(adev);
>>> -    if (r)
>>> -        return r;
>>> -
>>>      if (!amdgpu_emu_mode)
>>>          gfx_v10_0_init_golden_registers(adev);
>>>
>>> @@ -3871,7 +3834,6 @@ static int gfx_v10_0_hw_fini(void *handle)
>>>      }
>>>      gfx_v10_0_cp_enable(adev, false);
>>>      gfx_v10_0_enable_gui_idle_interrupt(adev, false);
>>> -    gfx_v10_0_csb_vram_unpin(adev);
>>>
>>>      return 0;
>>> }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> index 4cc2e50..524a7ba 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>> @@ -1683,39 +1683,6 @@ static int gfx_v9_0_rlc_init(struct amdgpu_device *adev)
>>>      return 0;
>>> }
>>>
>>> -static int gfx_v9_0_csb_vram_pin(struct amdgpu_device *adev) -{
>>> -    int r;
>>> -
>>> -    r = amdgpu_bo_reserve(adev->gfx.rlc.clear_state_obj, false);
>>> -    if (unlikely(r != 0))
>>> -        return r;
>>> -
>>> -    r = amdgpu_bo_pin(adev->gfx.rlc.clear_state_obj,
>>> -            AMDGPU_GEM_DOMAIN_VRAM);
>>> -    if (!r)
>>> -        adev->gfx.rlc.clear_state_gpu_addr =
>>> -            amdgpu_bo_gpu_offset(adev->gfx.rlc.clear_state_obj);
>>> -
>>> -    amdgpu_bo_unreserve(adev->gfx.rlc.clear_state_obj);
>>> -
>>> -    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); 
>>> @@
>>> -3694,10 +3661,6 @@ static int gfx_v9_0_hw_init(void *handle)
>>>
>>>      gfx_v9_0_constants_init(adev);
>>>
>>> -    r = gfx_v9_0_csb_vram_pin(adev);
>>> -    if (r)
>>> -        return r;
>>> -
>>>      r = adev->gfx.rlc.funcs->resume(adev);
>>>      if (r)
>>>          return r;
>>> @@ -3779,8 +3742,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@xxxxxxxxxxxxxxxxxxxxx
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fli
>>> s
>>> t
>>> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7C
>>> X
>>> i
>>> aojie.Yuan%40amd.com%7C65e162e509ea4a90f79308d77266de65%7C3dd8961fe4
>>> 8
>>> 8
>>> 4e608e11a82d994e183d%7C0%7C0%7C637103658464512751&amp;sdata=r5fpid5I
>>> s
>>> P
>>> 8anzg%2FZIYHn0N8xceBvG7rtRG80%2B7868o%3D&amp;reserved=0
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>> t 
>> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CM
>> o
>> nk.Liu%40amd.com%7Ccadef01b84ab45f90f1908d773f932cc%7C3dd8961fe4884e6
>> 0 
>> 8e11a82d994e183d%7C0%7C0%7C637105386456299680&amp;sdata=hYsvNtzUb%2BT
>> b
>> iANKAx2x9dmYW1ikC66r%2B6Hbk3244PE%3D&amp;reserved=0

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux