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

2021-10-21 Thread Christian König

Am 21.10.21 um 16:31 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.

v2: pass adev's gtt_mgr instead of adev

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  4 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 23 ++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  4 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 ++--
  4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 41ce86244144..2b53d86aebac 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(>mman.bdev, TTM_PL_TT));
+   amdgpu_gtt_mgr_recover(>mman.gtt_mgr);

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(_adev->mman.bdev, TTM_PL_TT));
+   r = 
amdgpu_gtt_mgr_recover(_adev->mman.gtt_mgr);
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..e429f2df73be 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(>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(>mman.gtt_mgr));
  }

  static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
@@ -206,14 +204,15 @@ 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
+ * @mgr: amdgpu_gtt_mgr 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_gtt_mgr *mgr)
  {
-   struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
-   s64 result = man->size - atomic64_read(>available);
+   s64 result;
+
+   result = mgr->manager.size - atomic64_read(>available);

return (result > 0 ? result : 0) * PAGE_SIZE;
  }
@@ -221,16 +220,15 @@ uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager 
*man)
  /**
   * amdgpu_gtt_mgr_recover - re-init gart
   *
- * @man: TTM memory type manager
+ * @mgr: amdgpu_gtt_mgr 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_gtt_mgr *mgr)
  {
-   struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
-   struct amdgpu_device *adev;
struct amdgpu_gtt_node *node;
struct drm_mm_node *mm_node;
+   struct amdgpu_device *adev;
int r = 0;

adev = container_of(mgr, typeof(*adev), mman.gtt_mgr);
@@ -260,6 +258,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(>lock);
drm_mm_print(>mm, printer);
@@ -267,7 +266,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(>available),
-  amdgpu_gtt_mgr_usage(man) >> 20);
+  amdgpu_gtt_mgr_usage(>mman.gtt_mgr) >> 20);


That here needs fixing, we shouldn't use the adev->mman.gtt_mgr here but 
rather upcast man.


Regards,
Christian.


  }

  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..603ce32db5c5 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 

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

2021-10-21 Thread 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.

v2: pass adev's gtt_mgr instead of adev

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 23 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  4 ++--
 4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 41ce86244144..2b53d86aebac 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(>mman.bdev, TTM_PL_TT));
+   amdgpu_gtt_mgr_recover(>mman.gtt_mgr);

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(_adev->mman.bdev, TTM_PL_TT));
+   r = 
amdgpu_gtt_mgr_recover(_adev->mman.gtt_mgr);
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..e429f2df73be 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(>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(>mman.gtt_mgr));
 }

 static DEVICE_ATTR(mem_info_gtt_total, S_IRUGO,
@@ -206,14 +204,15 @@ 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
+ * @mgr: amdgpu_gtt_mgr 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_gtt_mgr *mgr)
 {
-   struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
-   s64 result = man->size - atomic64_read(>available);
+   s64 result;
+
+   result = mgr->manager.size - atomic64_read(>available);

return (result > 0 ? result : 0) * PAGE_SIZE;
 }
@@ -221,16 +220,15 @@ uint64_t amdgpu_gtt_mgr_usage(struct ttm_resource_manager 
*man)
 /**
  * amdgpu_gtt_mgr_recover - re-init gart
  *
- * @man: TTM memory type manager
+ * @mgr: amdgpu_gtt_mgr 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_gtt_mgr *mgr)
 {
-   struct amdgpu_gtt_mgr *mgr = to_gtt_mgr(man);
-   struct amdgpu_device *adev;
struct amdgpu_gtt_node *node;
struct drm_mm_node *mm_node;
+   struct amdgpu_device *adev;
int r = 0;

adev = container_of(mgr, typeof(*adev), mman.gtt_mgr);
@@ -260,6 +258,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(>lock);
drm_mm_print(>mm, printer);
@@ -267,7 +266,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(>available),
-  amdgpu_gtt_mgr_usage(man) >> 20);
+  amdgpu_gtt_mgr_usage(>mman.gtt_mgr) >> 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..603ce32db5c5 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(>mman.bdev, TTM_PL_VRAM));
return copy_to_user(out, ,