Re: [PATCH 1/3] drm/amdgpu: do not pass ttm_resource_manager to gtt_mgr

Wed, 20 Oct 2021 04:12:25 -0700 


On 10/20/2021 12:49 PM, Christian König wrote:

Am 20.10.21 um 11:19 schrieb Lazar, Lijo:



On 10/20/2021 2:18 PM, Das, Nirmoy wrote:


On 10/20/2021 8:49 AM, Christian König wrote:

Am 19.10.21 um 20:14 schrieb Nirmoy Das:

Do not allow exported amdgpu_gtt_mgr_*() to accept
any ttm_resource_manager pointer. Also there is no need
to force other module to call a ttm function just to
eventually call gtt_mgr functions.


That's a rather bad idea I think.


The GTT and VRAM manager work on their respective objects and not 
on the adev directly.




What is bothering me is : it is obvious that  the 
amdgpu_gtt_mgr_usage() for example should only calculate



usages for TTM_PL_TT type resource manager, why to pass that 
explicitly. I am trying to leverage the fact that



we only have one gtt/vram manager for a adev and the functions that 
I changed  work on whole gtt/vram manager


as a unit.




Don't know about the functional aspects. From a sofware perspective, 
amdgpu_gtt_mgr_*() operating on struct amdgpu_gtt_mgr *mgr seems more 
logical.



What we could do is to pass in amdgpu_gtt_mgr instead of 
ttm_resource_manager and then use &adev->mman.gtt_mgr.



Sounds good, I will try this way then.


Regards,

Nirmoy



Regards,
Christian.




Thanks,
Lijo



Regards,

Nirmoy




Christian.



Signed-off-by: Nirmoy Das <nirmoy....@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  4 +--

  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 31 
++++++++++++---------

  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c     |  4 +--
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  4 +--
  4 files changed, 24 insertions(+), 19 deletions(-)


diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

index 41ce86244144..5807df52031c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

@@ -4287,7 +4287,7 @@ static int amdgpu_device_reset_sriov(struct 
amdgpu_device *adev,

        amdgpu_virt_init_data_exchange(adev);
      /* we need recover gart prior to run SMC/CP/SDMA resume */

- amdgpu_gtt_mgr_recover(ttm_manager_type(&adev->mman.bdev, 
TTM_PL_TT));

+    amdgpu_gtt_mgr_recover(adev);
        r = amdgpu_device_fw_loading(adev);
      if (r)

@@ -4604,7 +4604,7 @@ int amdgpu_do_asic_reset(struct list_head 
*device_list_handle,

                      amdgpu_inc_vram_lost(tmp_adev);
                  }

  -                r = 
amdgpu_gtt_mgr_recover(ttm_manager_type(&tmp_adev->mman.bdev, 
TTM_PL_TT));

+                r = amdgpu_gtt_mgr_recover(tmp_adev);
                  if (r)
                      goto out;

  diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c

index c18f16b3be9c..5e41f8ef743a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c

@@ -77,10 +77,8 @@ static ssize_t 
amdgpu_mem_info_gtt_used_show(struct device *dev,

  {
      struct drm_device *ddev = dev_get_drvdata(dev);
      struct amdgpu_device *adev = drm_to_adev(ddev);
-    struct ttm_resource_manager *man;
  -    man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
-    return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(man));
+    return sysfs_emit(buf, "%llu\n", amdgpu_gtt_mgr_usage(adev));
  }
    static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,

@@ -206,14 +204,19 @@ static void amdgpu_gtt_mgr_del(struct 
ttm_resource_manager *man,

  /**
   * amdgpu_gtt_mgr_usage - return usage of GTT domain
   *
- * @man: TTM memory type manager
+ * @adev: amdgpu_device pointer
   *
   * Return how many bytes are used in the GTT domain
   */
-uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager *man)
+uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_device *adev)
  {
-    struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
-    s64 result = man->size - atomic64_read(&mgr->available);
+    struct ttm_resource_manager *man;
+    struct amdgpu_gtt_mgr *mgr;
+    s64 result;
+
+    man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
+    mgr = to_gtt_mgr(man);
+    result = man->size - atomic64_read(&mgr->available);
        return (result > 0 ? result : 0) * PAGE_SIZE;
  }

@@ -221,19 +224,20 @@ uint64_t amdgpu_gtt_mgr_usage(struct 
ttm_resource_manager *man)

  /**
   * amdgpu_gtt_mgr_recover - re-init gart
   *
- * @man: TTM memory type manager
+ * @adev: amdgpu_device pointer
   *
   * Re-init the gart for each known BO in the GTT.
   */
-int amdgpu_gtt_mgr_recover(struct ttm_resource_manager *man)
+int amdgpu_gtt_mgr_recover(struct amdgpu_device *adev)
  {
-    struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
-    struct amdgpu_device *adev;
+    struct ttm_resource_manager *man;
+    struct amdgpu_gtt_mgr *mgr;
      struct amdgpu_gtt_node *node;
      struct drm_mm_node *mm_node;
      int r = 0;
  -    adev = container_of(mgr, typeof(*adev), mman.gtt_mgr);
+    man = ttm_manager_type(&adev->mman.bdev, TTM_PL_TT);
+    mgr = to_gtt_mgr(man);
      spin_lock(&mgr->lock);
      drm_mm_for_each_node(mm_node, &mgr->mm) {

          node = container_of(mm_node, typeof(*node), 
base.mm_nodes[0]);
@@ -260,6 +264,7 @@ static void amdgpu_gtt_mgr_debug(struct 
ttm_resource_manager *man,

                   struct drm_printer *printer)
  {
      struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);

+    struct amdgpu_device *adev = container_of(mgr, typeof(*adev), 
mman.gtt_mgr);

        spin_lock(&mgr->lock);
      drm_mm_print(&mgr->mm, printer);

@@ -267,7 +272,7 @@ static void amdgpu_gtt_mgr_debug(struct 
ttm_resource_manager *man,
        drm_printf(printer, "man size:%llu pages, gtt 
available:%lld pages, usage:%lluMB\n",

             man->size, (u64)atomic64_read(&mgr->available),
-           amdgpu_gtt_mgr_usage(man) >> 20);
+           amdgpu_gtt_mgr_usage(adev) >> 20);
  }

    static const struct ttm_resource_manager_func 
amdgpu_gtt_mgr_func = {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

index d2955ea4a62b..b9b38f70e416 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

@@ -678,7 +678,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, 
void *data, struct drm_file *filp)
          ui64 = 
amdgpu_vram_mgr_vis_usage(ttm_manager_type(&adev->mman.bdev, 
TTM_PL_VRAM));
          return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT 
: 0;

      case AMDGPU_INFO_GTT_USAGE:

-        ui64 = 
amdgpu_gtt_mgr_usage(ttm_manager_type(&adev->mman.bdev, TTM_PL_TT));

+        ui64 = amdgpu_gtt_mgr_usage(adev);

          return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT 
: 0;

      case AMDGPU_INFO_GDS_CONFIG: {
          struct drm_amdgpu_info_gds gds_info;

@@ -738,7 +738,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, 
void *data, struct drm_file *filp)

          mem.gtt.usable_heap_size = mem.gtt.total_heap_size -
              atomic64_read(&adev->gart_pin_size);
          mem.gtt.heap_usage =
-            amdgpu_gtt_mgr_usage(gtt_man);
+            amdgpu_gtt_mgr_usage(adev);
          mem.gtt.max_allocation = mem.gtt.usable_heap_size * 3 / 4;
            return copy_to_user(out, &mem,

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h

index 91a087f9dc7c..6e337cacef51 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h

@@ -114,8 +114,8 @@ int amdgpu_vram_mgr_init(struct amdgpu_device 
*adev);

  void amdgpu_vram_mgr_fini(struct amdgpu_device *adev);
    bool amdgpu_gtt_mgr_has_gart_addr(struct ttm_resource *mem);
-uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager *man);
-int amdgpu_gtt_mgr_recover(struct ttm_resource_manager *man);
+uint64_t amdgpu_gtt_mgr_usage(struct amdgpu_device *adev);
+int amdgpu_gtt_mgr_recover(struct amdgpu_device *adev);

    uint64_t amdgpu_preempt_mgr_usage(struct ttm_resource_manager 
*man);



























					Reply via email to