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

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

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 <Amaranath.Somalapuram@xxxxxxx>
> ---
>   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(&bo->tbo, 0, bo->tbo.resource->num_pages, &bo->kmap);
> +	r = ttm_bo_kmap(&bo->tbo, 0, PFN_UP(bo->tbo.resource->size), &bo->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: