Re: [Nouveau] [PATCH 5/6] drm/radeon: fix’s on ttm_resource rework to use size_t type

2022-10-19 Thread Christian König




Am 19.10.22 um 17:27 schrieb Somalapuram Amaranath:

Fix the ttm_resource from num_pages to size_t size.

Signed-off-by: Somalapuram Amaranath 
---
  drivers/gpu/drm/radeon/radeon_cs.c | 4 ++--
  drivers/gpu/drm/radeon/radeon_object.c | 4 ++--
  drivers/gpu/drm/radeon/radeon_trace.h  | 2 +-
  drivers/gpu/drm/radeon/radeon_ttm.c| 4 ++--
  4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_cs.c 
b/drivers/gpu/drm/radeon/radeon_cs.c
index 446f7bae54c4..4c930f0cf132 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -400,8 +400,8 @@ static int cmp_size_smaller_first(void *priv, const struct 
list_head *a,
struct radeon_bo_list *lb = list_entry(b, struct radeon_bo_list, 
tv.head);
  
  	/* Sort A before B if A is smaller. */

-   return (int)la->robj->tbo.resource->num_pages -
-   (int)lb->robj->tbo.resource->num_pages;
+   return (int)PFN_UP(la->robj->tbo.resource->size) -
+   (int)PFN_UP(lb->robj->tbo.resource->size);


I think you can drop the conversion and PFN_UP. What we need here is a 
compare result. Something like this:


if (la->robj->tbo.resource->size > lb->robj->tbo.resource->size)
    return 1;
if (la->robj->tbo.resource->size < lb->robj->tbo.resource->size)
    return -1;
return 0;

And I think it makes more sense to use tbo.base.size here as well 
instead of the resource size.


Regards,
Christian.


  }
  
  /**

diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
b/drivers/gpu/drm/radeon/radeon_object.c
index 00c33b24d5d3..710d04fcbea6 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -232,7 +232,7 @@ int radeon_bo_kmap(struct radeon_bo *bo, void **ptr)
}
return 0;
}
-   r = ttm_bo_kmap(>tbo, 0, bo->tbo.resource->num_pages, >kmap);
+   r = ttm_bo_kmap(>tbo, 0, PFN_UP(bo->tbo.resource->size), >kmap);
if (r) {
return r;
}
@@ -737,7 +737,7 @@ vm_fault_t radeon_bo_fault_reserve_notify(struct 
ttm_buffer_object *bo)
if (bo->resource->mem_type != TTM_PL_VRAM)
return 0;
  
-	size = bo->resource->num_pages << PAGE_SHIFT;

+   size = bo->resource->size;
offset = bo->resource->start << PAGE_SHIFT;
if ((offset + size) <= rdev->mc.visible_vram_size)
return 0;
diff --git a/drivers/gpu/drm/radeon/radeon_trace.h 
b/drivers/gpu/drm/radeon/radeon_trace.h
index c9fed5f2b870..22676617e1a5 100644
--- a/drivers/gpu/drm/radeon/radeon_trace.h
+++ b/drivers/gpu/drm/radeon/radeon_trace.h
@@ -22,7 +22,7 @@ TRACE_EVENT(radeon_bo_create,
  
  	TP_fast_assign(

   __entry->bo = bo;
-  __entry->pages = bo->tbo.resource->num_pages;
+  __entry->pages = PFN_UP(bo->tbo.resource->size);
   ),
TP_printk("bo=%p, pages=%u", __entry->bo, __entry->pages)
  );
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index d33fec488713..fff48306c05f 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -181,7 +181,7 @@ static int radeon_move_blit(struct ttm_buffer_object *bo,
  
  	BUILD_BUG_ON((PAGE_SIZE % RADEON_GPU_PAGE_SIZE) != 0);
  
-	num_pages = new_mem->num_pages * (PAGE_SIZE / RADEON_GPU_PAGE_SIZE);

+   num_pages = PFN_UP(new_mem->size) * (PAGE_SIZE / RADEON_GPU_PAGE_SIZE);
fence = radeon_copy(rdev, old_start, new_start, num_pages, 
bo->base.resv);
if (IS_ERR(fence))
return PTR_ERR(fence);
@@ -268,7 +268,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, 
bool evict,
  static int radeon_ttm_io_mem_reserve(struct ttm_device *bdev, struct 
ttm_resource *mem)
  {
struct radeon_device *rdev = radeon_get_rdev(bdev);
-   size_t bus_size = (size_t)mem->num_pages << PAGE_SHIFT;
+   size_t bus_size = (size_t)mem->size;
  
  	switch (mem->mem_type) {

case TTM_PL_SYSTEM:




Re: [Nouveau] [PATCH 4/6] drm/nouveau: fix’s on ttm_resource rework to use size_t type

2022-10-19 Thread Christian König

Am 19.10.22 um 17:27 schrieb Somalapuram Amaranath:

Fix the ttm_resource from num_pages to size_t size.

Signed-off-by: Somalapuram Amaranath 


I'm not an expert for nouveau so it might be possible that we better use 
bo->base.size instead of bo->resource->size at some places. But that can 
be cleaned up later if anybody really cares.


Acked-by: Christian König 


---
  drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++--
  drivers/gpu/drm/nouveau/nouveau_bo0039.c | 4 ++--
  drivers/gpu/drm/nouveau/nouveau_bo5039.c | 2 +-
  drivers/gpu/drm/nouveau/nouveau_bo74c1.c | 2 +-
  drivers/gpu/drm/nouveau/nouveau_bo85b5.c | 4 ++--
  drivers/gpu/drm/nouveau/nouveau_bo9039.c | 4 ++--
  drivers/gpu/drm/nouveau/nouveau_bo90b5.c | 4 ++--
  drivers/gpu/drm/nouveau/nouveau_boa0b5.c | 2 +-
  drivers/gpu/drm/nouveau/nouveau_gem.c| 5 ++---
  drivers/gpu/drm/nouveau/nouveau_mem.c| 4 ++--
  drivers/gpu/drm/nouveau/nouveau_ttm.c| 2 +-
  11 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 126b3c6e12f9..16ca4a141866 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -532,7 +532,7 @@ nouveau_bo_map(struct nouveau_bo *nvbo)
if (ret)
return ret;
  
-	ret = ttm_bo_kmap(>bo, 0, nvbo->bo.resource->num_pages, >kmap);

+   ret = ttm_bo_kmap(>bo, 0, PFN_UP(nvbo->bo.resource->size), 
>kmap);
  
  	ttm_bo_unreserve(>bo);

return ret;
@@ -1236,7 +1236,7 @@ vm_fault_t nouveau_ttm_fault_reserve_notify(struct 
ttm_buffer_object *bo)
} else {
/* make sure bo is in mappable vram */
if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_TESLA ||
-   bo->resource->start + bo->resource->num_pages < mappable)
+   bo->resource->start + PFN_UP(bo->resource->size) < mappable)
return 0;
  
  		for (i = 0; i < nvbo->placement.num_placement; ++i) {

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo0039.c 
b/drivers/gpu/drm/nouveau/nouveau_bo0039.c
index 7390132129fe..e2ce44adaa5c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo0039.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo0039.c
@@ -52,7 +52,7 @@ nv04_bo_move_m2mf(struct nouveau_channel *chan, struct 
ttm_buffer_object *bo,
u32 src_offset = old_reg->start << PAGE_SHIFT;
u32 dst_ctxdma = nouveau_bo_mem_ctxdma(bo, chan, new_reg);
u32 dst_offset = new_reg->start << PAGE_SHIFT;
-   u32 page_count = new_reg->num_pages;
+   u32 page_count = PFN_UP(new_reg->size);
int ret;
  
  	ret = PUSH_WAIT(push, 3);

@@ -62,7 +62,7 @@ nv04_bo_move_m2mf(struct nouveau_channel *chan, struct 
ttm_buffer_object *bo,
PUSH_MTHD(push, NV039, SET_CONTEXT_DMA_BUFFER_IN, src_ctxdma,
   SET_CONTEXT_DMA_BUFFER_OUT, dst_ctxdma);
  
-	page_count = new_reg->num_pages;

+   page_count = PFN_UP(new_reg->size);
while (page_count) {
int line_count = (page_count > 2047) ? 2047 : page_count;
  
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo5039.c b/drivers/gpu/drm/nouveau/nouveau_bo5039.c

index 4c75c7b3804c..c6cf3629a9f9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo5039.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo5039.c
@@ -41,7 +41,7 @@ nv50_bo_move_m2mf(struct nouveau_channel *chan, struct 
ttm_buffer_object *bo,
  {
struct nouveau_mem *mem = nouveau_mem(old_reg);
struct nvif_push *push = chan->chan.push;
-   u64 length = (new_reg->num_pages << PAGE_SHIFT);
+   u64 length = new_reg->size;
u64 src_offset = mem->vma[0].addr;
u64 dst_offset = mem->vma[1].addr;
int src_tiled = !!mem->kind;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo74c1.c 
b/drivers/gpu/drm/nouveau/nouveau_bo74c1.c
index ed6c09d67840..9b7ba31fae13 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo74c1.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo74c1.c
@@ -44,7 +44,7 @@ nv84_bo_move_exec(struct nouveau_channel *chan, struct 
ttm_buffer_object *bo,
if (ret)
return ret;
  
-	PUSH_NVSQ(push, NV74C1, 0x0304, new_reg->num_pages << PAGE_SHIFT,

+   PUSH_NVSQ(push, NV74C1, 0x0304, new_reg->size,
0x0308, upper_32_bits(mem->vma[0].addr),
0x030c, lower_32_bits(mem->vma[0].addr),
0x0310, upper_32_bits(mem->vma[1].addr),
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo85b5.c 
b/drivers/gpu/drm/nouveau/nouveau_bo85b5.c
index dec29b2d8bb2..a15a38a87a95 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo85b5.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo85b5.c
@@ -44,10 +44,10 @@ nva3_bo_move_copy(struct nouveau_channel *chan, struct 
ttm_buffer_object *bo,
struct nvif_push *push = chan->chan.push;
u64 src_offset = mem->vma[0].addr;
u64 dst_offset = mem->vma[1].addr;
-   u32 page_count = new_reg->num_pages;
+   u32 

Re: [Nouveau] [PATCH 3/6] drm/i915: fix’s on ttm_resource rework to use size_t type

2022-10-19 Thread Christian König

Am 19.10.22 um 17:27 schrieb Somalapuram Amaranath:

Fix the ttm_resource from num_pages to size_t size.

Signed-off-by: Somalapuram Amaranath 


Acked-by: Christian König 


---
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c   |  2 +-
  drivers/gpu/drm/i915/i915_scatterlist.c   |  4 ++--
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 12 ++--
  drivers/gpu/drm/i915/intel_region_ttm.c   |  2 +-
  4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c 
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 4f861782c3e8..7a1e92c11946 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -649,7 +649,7 @@ bool i915_ttm_resource_mappable(struct ttm_resource *res)
if (!i915_ttm_cpu_maps_iomem(res))
return true;
  
-	return bman_res->used_visible_size == bman_res->base.num_pages;

+   return bman_res->used_visible_size == PFN_UP(bman_res->base.size);
  }
  
  static int i915_ttm_io_mem_reserve(struct ttm_device *bdev, struct ttm_resource *mem)

diff --git a/drivers/gpu/drm/i915/i915_scatterlist.c 
b/drivers/gpu/drm/i915/i915_scatterlist.c
index dcc081874ec8..114e5e39aa72 100644
--- a/drivers/gpu/drm/i915/i915_scatterlist.c
+++ b/drivers/gpu/drm/i915/i915_scatterlist.c
@@ -158,7 +158,7 @@ struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct 
ttm_resource *res,
 u32 page_alignment)
  {
struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res);
-   const u64 size = res->num_pages << PAGE_SHIFT;
+   const u64 size = res->size;
const u32 max_segment = round_down(UINT_MAX, page_alignment);
struct drm_buddy *mm = bman_res->mm;
struct list_head *blocks = _res->blocks;
@@ -177,7 +177,7 @@ struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct 
ttm_resource *res,
  
  	i915_refct_sgt_init(rsgt, size);

st = >table;
-   if (sg_alloc_table(st, res->num_pages, GFP_KERNEL)) {
+   if (sg_alloc_table(st, PFN_UP(res->size), GFP_KERNEL)) {
i915_refct_sgt_put(rsgt);
return ERR_PTR(-ENOMEM);
}
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c 
b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
index e19452f0e100..7e611476c7a4 100644
--- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
+++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c
@@ -62,8 +62,8 @@ static int i915_ttm_buddy_man_alloc(struct 
ttm_resource_manager *man,
if (place->fpfn || lpfn != man->size)
bman_res->flags |= DRM_BUDDY_RANGE_ALLOCATION;
  
-	GEM_BUG_ON(!bman_res->base.num_pages);

-   size = bman_res->base.num_pages << PAGE_SHIFT;
+   GEM_BUG_ON(!bman_res->base.size);
+   size = bman_res->base.size;
  
  	min_page_size = bman->default_page_size;

if (bo->page_alignment)
@@ -72,7 +72,7 @@ static int i915_ttm_buddy_man_alloc(struct 
ttm_resource_manager *man,
GEM_BUG_ON(min_page_size < mm->chunk_size);
GEM_BUG_ON(!IS_ALIGNED(size, min_page_size));
  
-	if (place->fpfn + bman_res->base.num_pages != place->lpfn &&

+   if (place->fpfn + PFN_UP(bman_res->base.size) != place->lpfn &&
place->flags & TTM_PL_FLAG_CONTIGUOUS) {
unsigned long pages;
  
@@ -108,7 +108,7 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,

goto err_free_blocks;
  
  	if (place->flags & TTM_PL_FLAG_CONTIGUOUS) {

-   u64 original_size = (u64)bman_res->base.num_pages << PAGE_SHIFT;
+   u64 original_size = (u64)bman_res->base.size;
  
  		drm_buddy_block_trim(mm,

 original_size,
@@ -116,7 +116,7 @@ static int i915_ttm_buddy_man_alloc(struct 
ttm_resource_manager *man,
}
  
  	if (lpfn <= bman->visible_size) {

-   bman_res->used_visible_size = bman_res->base.num_pages;
+   bman_res->used_visible_size = PFN_UP(bman_res->base.size);
} else {
struct drm_buddy_block *block;
  
@@ -228,7 +228,7 @@ static bool i915_ttm_buddy_man_compatible(struct ttm_resource_manager *man,
  
  	if (!place->fpfn &&

place->lpfn == i915_ttm_buddy_man_visible_size(man))
-   return bman_res->used_visible_size == res->num_pages;
+   return bman_res->used_visible_size == PFN_UP(res->size);
  
  	/* Check each drm buddy block individually */

list_for_each_entry(block, _res->blocks, link) {
diff --git a/drivers/gpu/drm/i915/intel_region_ttm.c 
b/drivers/gpu/drm/i915/intel_region_ttm.c
index 575d67bc6ffe..cf89d0c2a2d9 100644
--- a/drivers/gpu/drm/i915/intel_region_ttm.c
+++ b/drivers/gpu/drm/i915/intel_region_ttm.c
@@ -244,7 +244,7 @@ void intel_region_ttm_resource_free(struct 
intel_memory_region *mem,
struct ttm_resource_manager *man = mem->region_private;
struct ttm_buffer_object mock_bo = {};
  
-	

Re: [Nouveau] [PATCH 2/6] drm/amd: fix’s on ttm_resource rework to use size_t type

2022-10-19 Thread Christian König

Am 19.10.22 um 17:27 schrieb Somalapuram Amaranath:

Fix the ttm_resource from num_pages to size_t size.

Signed-off-by: Somalapuram Amaranath 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c| 2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h | 4 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h  | 2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 6 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c   | 8 
  6 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 1f3302aebeff..44367f03316f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -144,7 +144,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager 
*man,
node->base.start = node->mm_nodes[0].start;
} else {
node->mm_nodes[0].start = 0;
-   node->mm_nodes[0].size = node->base.num_pages;
+   node->mm_nodes[0].size = PFN_UP(node->base.size);
node->base.start = AMDGPU_BO_INVALID_OFFSET;
}
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 2e8f6cd7a729..e51f80bb1d07 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -542,6 +542,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
/* GWS and OA don't need any alignment. */
page_align = bp->byte_align;
size <<= PAGE_SHIFT;
+
} else if (bp->domain & AMDGPU_GEM_DOMAIN_GDS) {
/* Both size and alignment must be a multiple of 4. */
page_align = ALIGN(bp->byte_align, 4);
@@ -776,7 +777,7 @@ int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr)
return 0;
}
  
-	r = ttm_bo_kmap(>tbo, 0, bo->tbo.resource->num_pages, >kmap);

+   r = ttm_bo_kmap(>tbo, 0, PFN_UP(bo->tbo.resource->size), >kmap);


Here bo->tbo.base.size would make more sense, but apart from that the 
patch looks good to me.


Regards,
Christian.


if (r)
return r;
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h

index 6546552e596c..5c4f93ee0c57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -62,7 +62,7 @@ static inline void amdgpu_res_first(struct ttm_resource *res,
if (!res)
goto fallback;
  
-	BUG_ON(start + size > res->num_pages << PAGE_SHIFT);

+   BUG_ON(start + size > res->size);
  
  	cur->mem_type = res->mem_type;
  
@@ -110,7 +110,7 @@ static inline void amdgpu_res_first(struct ttm_resource *res,

cur->size = size;
cur->remaining = size;
cur->node = NULL;
-   WARN_ON(res && start + size > res->num_pages << PAGE_SHIFT);
+   WARN_ON(res && start + size > res->size);
return;
  }
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h

index 5e6ddc7e101c..677ad2016976 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -127,7 +127,7 @@ TRACE_EVENT(amdgpu_bo_create,
  
  	TP_fast_assign(

   __entry->bo = bo;
-  __entry->pages = bo->tbo.resource->num_pages;
+  __entry->pages = PFN_UP(bo->tbo.resource->size);
   __entry->type = bo->tbo.resource->mem_type;
   __entry->prefer = bo->preferred_domains;
   __entry->allow = bo->allowed_domains;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index dc262d2c2925..36066965346f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -381,7 +381,7 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo,
dst.offset = 0;
  
  	r = amdgpu_ttm_copy_mem_to_mem(adev, , ,

-  new_mem->num_pages << PAGE_SHIFT,
+  new_mem->size,
   amdgpu_bo_encrypted(abo),
   bo->base.resv, );
if (r)
@@ -424,7 +424,7 @@ static int amdgpu_move_blit(struct ttm_buffer_object *bo,
  static bool amdgpu_mem_visible(struct amdgpu_device *adev,
   struct ttm_resource *mem)
  {
-   u64 mem_size = (u64)mem->num_pages << PAGE_SHIFT;
+   u64 mem_size = (u64)mem->size;
struct amdgpu_res_cursor cursor;
u64 end;
  
@@ -568,7 +568,7 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_device *bdev,

 struct ttm_resource *mem)
  {
struct amdgpu_device *adev = 

Re: [Nouveau] [PATCH 1/6] drm/ttm: rework on ttm_resource to use size_t type

2022-10-19 Thread Christian König

Am 19.10.22 um 17:27 schrieb Somalapuram Amaranath:

Change ttm_resource structure from num_pages to size_t size in bytes.


When you remove the num_pages field (instead of adding the size 
additionally) you need to change all drivers in one patch.


Otherwise the build would break in between patches and that's not 
something we can do.




Signed-off-by: Somalapuram Amaranath 
---
  drivers/gpu/drm/ttm/ttm_bo.c| 4 ++--
  drivers/gpu/drm/ttm/ttm_bo_util.c   | 6 +++---
  drivers/gpu/drm/ttm/ttm_bo_vm.c | 4 ++--
  drivers/gpu/drm/ttm/ttm_range_manager.c | 2 +-
  drivers/gpu/drm/ttm/ttm_resource.c  | 8 
  include/drm/ttm/ttm_resource.h  | 2 +-
  6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 7c8e8be774f1..394ccb13eaed 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -51,8 +51,8 @@ static void ttm_bo_mem_space_debug(struct ttm_buffer_object 
*bo,
struct ttm_resource_manager *man;
int i, mem_type;
  
-	drm_printf(, "No space for %p (%lu pages, %zuK, %zuM)\n",

-  bo, bo->resource->num_pages, bo->base.size >> 10,
+   drm_printf(, "No space for %p (%lu size, %zuK, %zuM)\n",
+  bo, bo->resource->size, bo->base.size >> 10,


Please just remove printing the resource size completely here.


   bo->base.size >> 20);
for (i = 0; i < placement->num_placement; i++) {
mem_type = placement->placement[i].mem_type;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index fa04e62202c1..da5493f789df 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -173,7 +173,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
  
  	clear = src_iter->ops->maps_tt && (!ttm || !ttm_tt_is_populated(ttm));

if (!(clear && ttm && !(ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC)))
-   ttm_move_memcpy(clear, dst_mem->num_pages, dst_iter, src_iter);
+   ttm_move_memcpy(clear, PFN_UP(dst_mem->size), dst_iter, 
src_iter);


Please use ttm->num_pages here (IIRC).

  
  	if (!src_iter->ops->maps_tt)

ttm_kmap_iter_linear_io_fini(&_src_iter.io, bdev, src_mem);
@@ -357,9 +357,9 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
  
  	map->virtual = NULL;

map->bo = bo;
-   if (num_pages > bo->resource->num_pages)
+   if (num_pages > PFN_UP(bo->resource->size))
return -EINVAL;
-   if ((start_page + num_pages) > bo->resource->num_pages)
+   if ((start_page + num_pages) > PFN_UP(bo->resource->size))
return -EINVAL;
  
  	ret = ttm_mem_io_reserve(bo->bdev, bo->resource);

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index 38119311284d..876e7d07273c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -217,7 +217,7 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf,
page_last = vma_pages(vma) + vma->vm_pgoff -
drm_vma_node_start(>base.vma_node);
  
-	if (unlikely(page_offset >= bo->resource->num_pages))

+   if (unlikely(page_offset >= PFN_UP(bo->resource->size)))


Please use bo->base.size here. The resource size can actually be larger 
than the BO size, but the extra space shouldn't be CPU map-able.



return VM_FAULT_SIGBUS;
  
  	prot = ttm_io_prot(bo, bo->resource, prot);

@@ -412,7 +412,7 @@ int ttm_bo_vm_access(struct vm_area_struct *vma, unsigned 
long addr,
 << PAGE_SHIFT);
int ret;
  
-	if (len < 1 || (offset + len) >> PAGE_SHIFT > bo->resource->num_pages)

+   if (len < 1 || (offset + len) > bo->resource->size)


Same here, please use bo->base.size.


return -EIO;
  
  	ret = ttm_bo_reserve(bo, true, false, NULL);

diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c 
b/drivers/gpu/drm/ttm/ttm_range_manager.c
index f7c16c46cfbc..0a8bc0b7f380 100644
--- a/drivers/gpu/drm/ttm/ttm_range_manager.c
+++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
@@ -83,7 +83,7 @@ static int ttm_range_man_alloc(struct ttm_resource_manager 
*man,
  
  	spin_lock(>lock);

ret = drm_mm_insert_node_in_range(mm, >mm_nodes[0],
- node->base.num_pages,
+ PFN_UP(node->base.size),
  bo->page_alignment, 0,
  place->fpfn, lpfn, mode);
spin_unlock(>lock);
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c 
b/drivers/gpu/drm/ttm/ttm_resource.c
index a729c32a1e48..f9cce0727d40 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -177,7 +177,7 @@ void ttm_resource_init(struct ttm_buffer_object *bo,
struct ttm_resource_manager *man;
  
  	res->start = 0;

-   res->num_pages = 

[Nouveau] [PATCH] nouveau: Fix migrate_to_ram() for faulting page

2022-10-19 Thread Alistair Popple
Commit 16ce101db85d ("mm/memory.c: fix race when faulting a device private
page") changed the migrate_to_ram() callback to take a reference on the
device page to ensure it can't be freed while handling the fault.
Unfortunately the corresponding update to Nouveau to accommodate this
change was inadvertently dropped from that patch causing GPU to CPU
migration to fail so add it here.

Signed-off-by: Alistair Popple 
Fixes: 16ce101db85d ("mm/memory.c: fix race when faulting a device private 
page")
Cc: John Hubbard 
Cc: Ralph Campbell 
Cc: Lyude Paul 
Cc: Ben Skeggs 

---

Hi Andrew/Ben, apologies I must have accidentally dropped this small hunk
when rebasing prior to sending v2 of the original series. Without this
migration from GPU to CPU won't work in Nouveau so hopefully one of you can
take this for v6.1-rcX. Thanks.
---
 drivers/gpu/drm/nouveau/nouveau_dmem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c 
b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 5fe209107246..20fe53815b20 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -176,6 +176,7 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct 
vm_fault *vmf)
.src= ,
.dst= ,
.pgmap_owner= drm->dev,
+   .fault_page = vmf->page,
.flags  = MIGRATE_VMA_SELECT_DEVICE_PRIVATE,
};
 
-- 
2.35.1



Re: [Nouveau] [PATCH v5 12/22] drm/connector: Add a function to lookup a TV mode by its name

2022-10-19 Thread Noralf Trønnes



Den 19.10.2022 10.48, skrev Maxime Ripard:
> On Tue, Oct 18, 2022 at 02:29:00PM +0200, Noralf Trønnes wrote:
>>
>>
>> Den 18.10.2022 11.33, skrev Maxime Ripard:
>>> On Mon, Oct 17, 2022 at 12:44:45PM +0200, Noralf Trønnes wrote:
 Den 13.10.2022 15.18, skrev Maxime Ripard:
> As part of the command line parsing rework coming in the next patches,
> we'll need to lookup drm_connector_tv_mode values by their name, already
> defined in drm_tv_mode_enum_list.
>
> In order to avoid any code duplication, let's do a function that will
> perform a lookup of a TV mode name and return its value.
>
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_connector.c | 24 
>  include/drm/drm_connector.h |  2 ++
>  2 files changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c 
> b/drivers/gpu/drm/drm_connector.c
> index 820f4c730b38..30611c616435 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -991,6 +991,30 @@ static const struct drm_prop_enum_list 
> drm_tv_mode_enum_list[] = {
>  };
>  DRM_ENUM_NAME_FN(drm_get_tv_mode_name, drm_tv_mode_enum_list)
>  
> +/**
> + * drm_get_tv_mode_from_name - Translates a TV mode name into its enum 
> value
> + * @name: TV Mode name we want to convert
> + * @len: Length of @name
> + *
> + * Translates @name into an enum drm_connector_tv_mode.
> + *
> + * Returns: the enum value on success, a negative errno otherwise.
> + */
> +int drm_get_tv_mode_from_name(const char *name, size_t len)

 Do we really need to pass in length here? item->name has to always be
 NUL terminated otherwise things would break elsewhere, so it shouldn't
 be necessary AFAICS.
>>>
>>> The only user so far is the command-line parsing code, and we might very
>>> well have an option after the tv_mode, something like
>>> 720x480i,tv_mode=NTSC,rotate=180
>>>
>>> In this case, we won't get a NULL-terminated name.
>>
>> My point is that item->name will always be NUL terminated so strcmp()
>> will never look past that.
> 
> Right, but we don't have the guarantee that strlen(item->name) <
> strlen(name), and we could thus just access after the end of our name
> 

Ok, using the length limiting str funtions is the safe thing to do, so
len needs to stay. But I don't get the 'strlen(item->name) == len'
check. strncmp() will stop when it reaches a NUL in either string so no
need for the length check?

Anyways:

Reviewed-by: Noralf Trønnes 


Re: [Nouveau] [PATCH v5 12/22] drm/connector: Add a function to lookup a TV mode by its name

2022-10-19 Thread Maxime Ripard
On Tue, Oct 18, 2022 at 02:29:00PM +0200, Noralf Trønnes wrote:
> 
> 
> Den 18.10.2022 11.33, skrev Maxime Ripard:
> > On Mon, Oct 17, 2022 at 12:44:45PM +0200, Noralf Trønnes wrote:
> >> Den 13.10.2022 15.18, skrev Maxime Ripard:
> >>> As part of the command line parsing rework coming in the next patches,
> >>> we'll need to lookup drm_connector_tv_mode values by their name, already
> >>> defined in drm_tv_mode_enum_list.
> >>>
> >>> In order to avoid any code duplication, let's do a function that will
> >>> perform a lookup of a TV mode name and return its value.
> >>>
> >>> Signed-off-by: Maxime Ripard 
> >>> ---
> >>>  drivers/gpu/drm/drm_connector.c | 24 
> >>>  include/drm/drm_connector.h |  2 ++
> >>>  2 files changed, 26 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_connector.c 
> >>> b/drivers/gpu/drm/drm_connector.c
> >>> index 820f4c730b38..30611c616435 100644
> >>> --- a/drivers/gpu/drm/drm_connector.c
> >>> +++ b/drivers/gpu/drm/drm_connector.c
> >>> @@ -991,6 +991,30 @@ static const struct drm_prop_enum_list 
> >>> drm_tv_mode_enum_list[] = {
> >>>  };
> >>>  DRM_ENUM_NAME_FN(drm_get_tv_mode_name, drm_tv_mode_enum_list)
> >>>  
> >>> +/**
> >>> + * drm_get_tv_mode_from_name - Translates a TV mode name into its enum 
> >>> value
> >>> + * @name: TV Mode name we want to convert
> >>> + * @len: Length of @name
> >>> + *
> >>> + * Translates @name into an enum drm_connector_tv_mode.
> >>> + *
> >>> + * Returns: the enum value on success, a negative errno otherwise.
> >>> + */
> >>> +int drm_get_tv_mode_from_name(const char *name, size_t len)
> >>
> >> Do we really need to pass in length here? item->name has to always be
> >> NUL terminated otherwise things would break elsewhere, so it shouldn't
> >> be necessary AFAICS.
> > 
> > The only user so far is the command-line parsing code, and we might very
> > well have an option after the tv_mode, something like
> > 720x480i,tv_mode=NTSC,rotate=180
> > 
> > In this case, we won't get a NULL-terminated name.
>
> My point is that item->name will always be NUL terminated so strcmp()
> will never look past that.

Right, but we don't have the guarantee that strlen(item->name) <
strlen(name), and we could thus just access after the end of our name

Maxime


signature.asc
Description: PGP signature