Re: [PATCH] drm/amdgpu: cleanup ttm debug sdma vram access function

[Christian König]

Am 12.01.22 um 16:59 schrieb Jonathan Kim:
Some suggested cleanups to declutter ttm when doing debug VRAM access over
SDMA.

Signed-off-by: Jonathan Kim <jonathan....@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  9 +++++++++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 23 +++++++----------------
  2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a675dde81ce0..4d77842f2183 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1448,6 +1448,15 @@ int amdgpu_device_set_cg_state(struct amdgpu_device 
*adev,
  int amdgpu_device_set_pg_state(struct amdgpu_device *adev,
                               enum amd_powergating_state state);
  
+static inline bool amdgpu_allow_post_mortem_debug(struct amdgpu_device *adev)

Give that a better name, something like 
amdgpu_device_are_timeouts_enabled().

Apart from that looks good to me,
Christian.

+{
+       return amdgpu_gpu_recovery == 0 ||
+               adev->gfx_timeout == MAX_SCHEDULE_TIMEOUT ||
+               adev->compute_timeout == MAX_SCHEDULE_TIMEOUT ||
+               adev->sdma_timeout == MAX_SCHEDULE_TIMEOUT ||
+               adev->video_timeout == MAX_SCHEDULE_TIMEOUT;
+}
+
  #include "amdgpu_object.h"
  
  static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 33781509838c..02515f1ea5fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1460,10 +1460,11 @@ static int amdgpu_ttm_access_memory_sdma(struct 
ttm_buffer_object *bo,
        if (r)
                goto out;
  
-	src_addr = write ? amdgpu_bo_gpu_offset(adev->mman.sdma_access_bo) :
-                       amdgpu_bo_gpu_offset(abo);
-       dst_addr = write ? amdgpu_bo_gpu_offset(abo) :
-                       amdgpu_bo_gpu_offset(adev->mman.sdma_access_bo);
+       src_addr = amdgpu_bo_gpu_offset(abo);
+       dst_addr = amdgpu_bo_gpu_offset(adev->mman.sdma_access_bo);
+       if (write)
+               swap(src_addr, dst_addr);
+
        amdgpu_emit_copy_buffer(adev, &job->ibs[0], src_addr, dst_addr, 
PAGE_SIZE, false);
  
  	amdgpu_ring_pad_ib(adev->mman.buffer_funcs_ring, &job->ibs[0]);
@@ -1486,15 +1487,6 @@ static int amdgpu_ttm_access_memory_sdma(struct 
ttm_buffer_object *bo,
        return r;
  }
  
-static inline bool amdgpu_ttm_allow_post_mortem_debug(struct amdgpu_device *adev)
-{
-       return amdgpu_gpu_recovery == 0 ||
-               adev->gfx_timeout == MAX_SCHEDULE_TIMEOUT ||
-               adev->compute_timeout == MAX_SCHEDULE_TIMEOUT ||
-               adev->sdma_timeout == MAX_SCHEDULE_TIMEOUT ||
-               adev->video_timeout == MAX_SCHEDULE_TIMEOUT;
-}
-
  /**
   * amdgpu_ttm_access_memory - Read or Write memory that backs a buffer object.
   *
@@ -1519,7 +1511,7 @@ static int amdgpu_ttm_access_memory(struct 
ttm_buffer_object *bo,
        if (bo->resource->mem_type != TTM_PL_VRAM)
                return -EIO;
  
-	if (!amdgpu_ttm_allow_post_mortem_debug(adev) &&
+       if (!amdgpu_allow_post_mortem_debug(adev) &&
                        !amdgpu_ttm_access_memory_sdma(bo, offset, buf, len, 
write))
                return len;
  
@@ -1909,8 +1901,7 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
        ttm_range_man_fini(&adev->mman.bdev, AMDGPU_PL_OA);
        ttm_device_fini(&adev->mman.bdev);
        adev->mman.initialized = false;
-       if (adev->mman.sdma_access_ptr)
-               amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
+       amdgpu_bo_free_kernel(&adev->mman.sdma_access_bo, NULL,
                                        &adev->mman.sdma_access_ptr);
        DRM_INFO("amdgpu: ttm finalized\n");
  }