Re: [RFC PATCH v2] drm/amdgpu: move ttm bo->offset to amdgpu_bo

2020-02-11 Thread Christian König

Am 11.02.20 um 15:54 schrieb Nirmoy:



On 2/11/20 3:09 PM, Christian König wrote:

Am 11.02.20 um 15:06 schrieb Nirmoy Das:

GPU address should belong to driver not in memory management.
This patch moves ttm bo.offset and gpu_offset calculation to amdgpu 
driver.


Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 
+++--

  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  8 +---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 ++--
  4 files changed, 24 insertions(+), 11 deletions(-)

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

index 6f60a581e3ba..e25924be33ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -917,7 +917,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo 
*bo, u32 domain,

  bo->pin_count++;
    if (max_offset != 0) {
-    u64 domain_start = bo->tbo.bdev->man[mem_type].gpu_offset;
+    u64 domain_start = amdgpu_bo_gpu_start(adev, mem_type);
  WARN_ON_ONCE(max_offset <
   (amdgpu_bo_gpu_offset(bo) - domain_start));
  }
@@ -1248,6 +1248,18 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo 
*bo, void *buffer,

  return 0;
  }
  +uint64_t amdgpu_bo_gpu_start(struct amdgpu_device *adev, uint32_t 
type)


Probably better to put that into amdgpu_ttm.[ch] or amdgpu_gmc.[ch] 
and name it an amdgpu_ttm_domain_start.



+{
+    switch(type) {
+    case TTM_PL_TT:


Coding style here is of, what editor and setting do you use?

vi with *set tabstop=8 softtabstop=8 shiftwidth=8 noexpandtab cindent*


My bad, I didn't check existing switch style.



Try this one here: https://github.com/vivien/vim-linux-coding-style

That should help,
Christian.



Regards,

Nirmoy


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [RFC PATCH v2] drm/amdgpu: move ttm bo->offset to amdgpu_bo

2020-02-11 Thread Nirmoy


On 2/11/20 3:09 PM, Christian König wrote:

Am 11.02.20 um 15:06 schrieb Nirmoy Das:

GPU address should belong to driver not in memory management.
This patch moves ttm bo.offset and gpu_offset calculation to amdgpu 
driver.


Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  8 +---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 ++--
  4 files changed, 24 insertions(+), 11 deletions(-)

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

index 6f60a581e3ba..e25924be33ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -917,7 +917,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo 
*bo, u32 domain,

  bo->pin_count++;
    if (max_offset != 0) {
-    u64 domain_start = bo->tbo.bdev->man[mem_type].gpu_offset;
+    u64 domain_start = amdgpu_bo_gpu_start(adev, mem_type);
  WARN_ON_ONCE(max_offset <
   (amdgpu_bo_gpu_offset(bo) - domain_start));
  }
@@ -1248,6 +1248,18 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo 
*bo, void *buffer,

  return 0;
  }
  +uint64_t amdgpu_bo_gpu_start(struct amdgpu_device *adev, uint32_t 
type)


Probably better to put that into amdgpu_ttm.[ch] or amdgpu_gmc.[ch] 
and name it an amdgpu_ttm_domain_start.



+{
+    switch(type) {
+    case TTM_PL_TT:


Coding style here is of, what editor and setting do you use?

vi with *set tabstop=8 softtabstop=8 shiftwidth=8 noexpandtab cindent*


My bad, I didn't check existing switch style.


Regards,

Nirmoy

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [RFC PATCH v2] drm/amdgpu: move ttm bo->offset to amdgpu_bo

2020-02-11 Thread Christian König

Am 11.02.20 um 15:06 schrieb Nirmoy Das:

GPU address should belong to driver not in memory management.
This patch moves ttm bo.offset and gpu_offset calculation to amdgpu driver.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |  8 +---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 ++--
  4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6f60a581e3ba..e25924be33ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -917,7 +917,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
domain,
bo->pin_count++;
  
  		if (max_offset != 0) {

-   u64 domain_start = 
bo->tbo.bdev->man[mem_type].gpu_offset;
+   u64 domain_start = amdgpu_bo_gpu_start(adev, mem_type);
WARN_ON_ONCE(max_offset <
 (amdgpu_bo_gpu_offset(bo) - domain_start));
}
@@ -1248,6 +1248,18 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void 
*buffer,
return 0;
  }
  
+uint64_t amdgpu_bo_gpu_start(struct amdgpu_device *adev, uint32_t type)


Probably better to put that into amdgpu_ttm.[ch] or amdgpu_gmc.[ch] and 
name it an amdgpu_ttm_domain_start.



+{
+   switch(type) {
+   case TTM_PL_TT:


Coding style here is of, what editor and setting do you use?


+   return adev->gmc.gart_start;
+   case TTM_PL_VRAM:
+   return adev->gmc.vram_start;
+   }
+
+   return 0;
+}
+
  /**
   * amdgpu_bo_move_notify - notification about a memory move
   * @bo: pointer to a buffer object
@@ -1460,6 +1472,9 @@ int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void 
*owner, bool intr)
   */
  u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
  {
+   uint64_t offset;
+   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);


Better put adev first.


+
WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM);
WARN_ON_ONCE(!dma_resv_is_locked(bo->tbo.base.resv) &&
 !bo->pin_count && bo->tbo.type != ttm_bo_type_kernel);
@@ -1467,7 +1482,10 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_VRAM &&
 !(bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS));
  
-	return amdgpu_gmc_sign_extend(bo->tbo.offset);

+offset = (bo->tbo.mem.start << PAGE_SHIFT) +
+amdgpu_bo_gpu_start(adev, bo->tbo.mem.mem_type);
+
+   return amdgpu_gmc_sign_extend(offset);
  }
  
  /**

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 96d805889e8d..dfa4e75b95e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -294,6 +294,7 @@ int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
  uint32_t amdgpu_bo_get_preferred_pin_domain(struct amdgpu_device *adev,
uint32_t domain);
  
+uint64_t amdgpu_bo_gpu_start(struct amdgpu_device *adev, uint32_t type);

  /*
   * sub allocation
   */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2c1d1eb1a7e1..3e65bdfca94f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -103,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
case TTM_PL_TT:
/* GTT memory  */
man->func = _gtt_mgr_func;
-   man->gpu_offset = adev->gmc.gart_start;
man->available_caching = TTM_PL_MASK_CACHING;
man->default_caching = TTM_PL_FLAG_CACHED;
man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -111,7 +110,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
case TTM_PL_VRAM:
/* "On-card" video ram */
man->func = _vram_mgr_func;
-   man->gpu_offset = adev->gmc.vram_start;
man->flags = TTM_MEMTYPE_FLAG_FIXED |
 TTM_MEMTYPE_FLAG_MAPPABLE;
man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
@@ -122,7 +120,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
case AMDGPU_PL_OA:
/* On-chip GDS memory*/
man->func = _bo_manager_func;
-   man->gpu_offset = 0;
man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_CMA;
man->available_caching = TTM_PL_FLAG_UNCACHED;
man->default_caching = TTM_PL_FLAG_UNCACHED;
@@ -270,7 +267,7 @@ static