Re: [PATCH 12/13] drm/ttm: remove io_ field from TTM V4

[Thomas Hellstrom <thomas@shipmail.org>]

Jerome Glisse wrote:
> All TTM driver have been converted to new io_mem_reserve/free
> interface which allow driver to choose and return proper io
> base, offset to core TTM for ioremapping if necessary. This
> patch remove what is now deadcode.
>
> V2 adapt to match with change in first patch of the patchset
> V3 update after io_mem_reserve/io_mem_free callback balancing
> V4 adjust to minor cleanup
>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
>   

Jerome, see comment below, apart from that, this is
Acked-by: Thomas Hellstrom <thellstrom@vmware.com>


> ---
>  drivers/gpu/drm/ttm/ttm_bo.c      |   22 ----------------------
>  drivers/gpu/drm/ttm/ttm_bo_util.c |   29 +++++++++--------------------
>  include/drm/ttm/ttm_bo_driver.h   |   10 ----------
>  3 files changed, 9 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 2171f86..1f27cf2 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -79,8 +79,6 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, int mem_type)
>  	printk(KERN_ERR TTM_PFX "    use_type: %d\n", man->use_type);
>  	printk(KERN_ERR TTM_PFX "    flags: 0x%08X\n", man->flags);
>  	printk(KERN_ERR TTM_PFX "    gpu_offset: 0x%08lX\n", man->gpu_offset);
> -	printk(KERN_ERR TTM_PFX "    io_offset: 0x%08lX\n", man->io_offset);
> -	printk(KERN_ERR TTM_PFX "    io_size: %ld\n", man->io_size);
>  	printk(KERN_ERR TTM_PFX "    size: %llu\n", man->size);
>  	printk(KERN_ERR TTM_PFX "    available_caching: 0x%08X\n",
>  		man->available_caching);
> @@ -1563,26 +1561,6 @@ bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>  	return true;
>  }
>  
> -int ttm_bo_pci_offset(struct ttm_bo_device *bdev,
> -		      struct ttm_mem_reg *mem,
> -		      unsigned long *bus_base,
> -		      unsigned long *bus_offset, unsigned long *bus_size)
> -{
> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
> -
> -	*bus_size = 0;
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> -		return -EINVAL;
> -
> -	if (ttm_mem_reg_is_pci(bdev, mem)) {
> -		*bus_offset = mem->mm_node->start << PAGE_SHIFT;
> -		*bus_size = mem->num_pages << PAGE_SHIFT;
> -		*bus_base = man->io_offset + (uintptr_t)man->io_addr;
> -	}
> -
> -	return 0;
> -}
> -
>  void ttm_bo_unmap_virtual(struct ttm_buffer_object *bo)
>  {
>  	struct ttm_bo_device *bdev = bo->bdev;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 878dc49..9f9b287 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -83,31 +83,20 @@ EXPORT_SYMBOL(ttm_bo_move_ttm);
>  
>  int ttm_mem_io_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>  {
> -	int ret;
> +	int ret = 0;
>  
> -	if (bdev->driver->io_mem_reserve) {
> -		if (!mem->bus.io_reserved) {
> -			mem->bus.io_reserved = true;
> -			ret = bdev->driver->io_mem_reserve(bdev, mem);
> -			if (unlikely(ret != 0))
> -				return ret;
> -		}
> -	} else {
> -		ret = ttm_bo_pci_offset(bdev, mem, &mem->bus.base, &mem->bus.offset, &mem->bus.size);
> -		if (unlikely(ret != 0))
> -			return ret;
> -		mem->bus.is_iomem = (mem->bus.size > 0) ? 1 : 0;
> +	if (!mem->bus.io_reserved) {
> +		mem->bus.io_reserved = true;
> +		ret = bdev->driver->io_mem_reserve(bdev, mem);
>  	}
> -	return 0;
> +	return ret;
>  }
>  
>  void ttm_mem_io_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>  {
> -	if (bdev->driver->io_mem_reserve) {
> -		if (mem->bus.io_reserved) {
> -			mem->bus.io_reserved = false;
> -			bdev->driver->io_mem_free(bdev, mem);
> -		}
> +	if (mem->bus.io_reserved) {
> +		mem->bus.io_reserved = false;
> +		bdev->driver->io_mem_free(bdev, mem);
>  	}
>  }
>  
> @@ -411,7 +400,7 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
>  
>  	if (!(man->flags & TTM_MEMTYPE_FLAG_NEEDS_IOREMAP)) {
>  		map->bo_kmap_type = ttm_bo_map_premapped;
> -		map->virtual = (void *)(((u8 *) man->io_addr) + bus_offset);
> +		map->virtual = (void *)(bus_base + bus_offset);
>   

I think we should completely remove TTM_MEMTYPE_FLAG_NEEDS_IOREMAP. With 
the new io_mem_reserve api that could be handled within the driver. I 
don't think there are drivers using this flag yet, and besides, i don't 
think the above code is correct anyway.



------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
--