[PATCH] drm/amdgpu: fix calltrace during kmd unload(v2)

Liu, Monk Monk.Liu at amd.com
Fri Nov 29 06:43:11 UTC 2019


@Koenig, Christian @Deucher, Alexander at Zhang, Hawking

Can you guys help to review the patch for me ? thanks for Xiaojie who helped to verify the S3 issue testing against this patch 

_____________________________________
Monk Liu|GPU Virtualization Team |AMD


-----Original Message-----
From: Yuan, Xiaojie <Xiaojie.Yuan at amd.com> 
Sent: Friday, November 29, 2019 2:31 PM
To: Liu, Monk <Monk.Liu at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: fix calltrace during kmd unload(v2)

[AMD Official Use Only - Internal Distribution Only]

Tested on navi14 and s3/baco works fine with patch applied.

Reviewed-by: Xiaojie Yuan <xiaojie.yuan at amd.com>

BR,
Xiaojie

________________________________________
From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> on behalf of Monk Liu <Monk.Liu at amd.com>
Sent: Thursday, November 28, 2019 2:57 PM
To: amd-gfx at lists.freedesktop.org
Cc: Liu, Monk
Subject: [PATCH] drm/amdgpu: fix calltrace during kmd unload(v2)

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

v2:
get_csb in init_rlc so hw_init() will make CSIB content back even after reset or s3.
take care of gfx7/8 as well

v3:
use bo_create_kernel instead of bo_create_reserved for CSB otherwise the bo_free_kernel() on CSB is not aligned and would led to its internal reserve pending there forever

Signed-off-by: Monk Liu <Monk.Liu at amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c | 10 +-----  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  | 58 +--------------------------------
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   |  2 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 40 +----------------------
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 40 +----------------------
 5 files changed, 6 insertions(+), 144 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
index c8793e6..6373bfb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.c
@@ -124,13 +124,12 @@ int amdgpu_gfx_rlc_init_sr(struct amdgpu_device *adev, u32 dws)
  */
 int amdgpu_gfx_rlc_init_csb(struct amdgpu_device *adev)  {
-       volatile u32 *dst_ptr;
        u32 dws;
        int r;

        /* allocate clear state block */
        adev->gfx.rlc.clear_state_size = dws = adev->gfx.rlc.funcs->get_csb_size(adev);
-       r = amdgpu_bo_create_reserved(adev, dws * 4, PAGE_SIZE,
+       r = amdgpu_bo_create_kernel(adev, dws * 4, PAGE_SIZE,
                                      AMDGPU_GEM_DOMAIN_VRAM,
                                      &adev->gfx.rlc.clear_state_obj,
                                      &adev->gfx.rlc.clear_state_gpu_addr,
@@ -141,13 +140,6 @@ int amdgpu_gfx_rlc_init_csb(struct amdgpu_device *adev)
                return r;
        }

-       /* set up the cs buffer */
-       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 7372904..7703b25 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -991,39 +991,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); @@ -1785,25 +1752,7 @@ static void gfx_v10_0_enable_gui_idle_interrupt(struct amdgpu_device *adev,

 static int gfx_v10_0_init_csb(struct amdgpu_device *adev)  {
-       int r;
-
-       if (adev->in_gpu_reset) {
-               r = amdgpu_bo_reserve(adev->gfx.rlc.clear_state_obj, false);
-               if (r)
-                       return r;
-
-               r = amdgpu_bo_kmap(adev->gfx.rlc.clear_state_obj,
-                                  (void **)&adev->gfx.rlc.cs_ptr);
-               if (!r) {
-                       adev->gfx.rlc.funcs->get_csb_buffer(adev,
-                                       adev->gfx.rlc.cs_ptr);
-                       amdgpu_bo_kunmap(adev->gfx.rlc.clear_state_obj);
-               }
-
-               amdgpu_bo_unreserve(adev->gfx.rlc.clear_state_obj);
-               if (r)
-                       return r;
-       }
+       adev->gfx.rlc.funcs->get_csb_buffer(adev, adev->gfx.rlc.cs_ptr);

        /* csib */
        WREG32_SOC15(GC, 0, mmRLC_CSIB_ADDR_HI, @@ -3774,10 +3723,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);

@@ -3865,7 +3810,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_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 43ae8fc..648d767 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -4554,6 +4554,8 @@ static int gfx_v7_0_hw_init(void *handle)

        gfx_v7_0_constants_init(adev);

+       /* init CSB */
+       adev->gfx.rlc.funcs->get_csb_buffer(adev, adev->gfx.rlc.cs_ptr);
        /* init rlc */
        r = adev->gfx.rlc.funcs->resume(adev);
        if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 80b7958..33c4ee9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -1321,39 +1321,6 @@ static int gfx_v8_0_rlc_init(struct amdgpu_device *adev)
        return 0;
 }

-static int gfx_v8_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_v8_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_v8_0_mec_fini(struct amdgpu_device *adev)  {
        amdgpu_bo_free_kernel(&adev->gfx.mec.hpd_eop_obj, NULL, NULL); @@ -3916,6 +3883,7 @@ static void gfx_v8_0_enable_gui_idle_interrupt(struct amdgpu_device *adev,

 static void gfx_v8_0_init_csb(struct amdgpu_device *adev)  {
+       adev->gfx.rlc.funcs->get_csb_buffer(adev, adev->gfx.rlc.cs_ptr);
        /* csib */
        WREG32(mmRLC_CSIB_ADDR_HI,
                        adev->gfx.rlc.clear_state_gpu_addr >> 32); @@ -4836,10 +4804,6 @@ static int gfx_v8_0_hw_init(void *handle)
        gfx_v8_0_init_golden_registers(adev);
        gfx_v8_0_constants_init(adev);

-       r = gfx_v8_0_csb_vram_pin(adev);
-       if (r)
-               return r;
-
        r = adev->gfx.rlc.funcs->resume(adev);
        if (r)
                return r;
@@ -4957,8 +4921,6 @@ static int gfx_v8_0_hw_fini(void *handle)
                pr_err("rlc is busy, skip halt rlc\n");
        amdgpu_gfx_rlc_exit_safe_mode(adev);

-       gfx_v8_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 e905ac5..6a251a3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1685,39 +1685,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); @@ -2405,6 +2372,7 @@ static void gfx_v9_0_enable_gui_idle_interrupt(struct amdgpu_device *adev,

 static void gfx_v9_0_init_csb(struct amdgpu_device *adev)  {
+       adev->gfx.rlc.funcs->get_csb_buffer(adev, adev->gfx.rlc.cs_ptr);
        /* csib */
        WREG32_RLC(SOC15_REG_OFFSET(GC, 0, mmRLC_CSIB_ADDR_HI),
                        adev->gfx.rlc.clear_state_gpu_addr >> 32); @@ -3696,10 +3664,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;
@@ -3781,8 +3745,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 at lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CXiaojie.Yuan%40amd.com%7C4773ce17189f4fa83aaf08d773d051a0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637105210873784154&sdata=OOr3UKdHl0Py0sYmsNX2t5UT4E1fbxKjRHxzZeyUtc8%3D&reserved=0



More information about the amd-gfx mailing list