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

["Yuan, Xiaojie" <Xiaojie.Yuan@xxxxxxx>]

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%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CXiaojie.Yuan%40amd.com%7C65e162e509ea4a90f79308d77266de65%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637103658464512751&amp;sdata=r5fpid5IsP8anzg%2FZIYHn0N8xceBvG7rtRG80%2B7868o%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx