diff mbox series

drm/ttm: remove special handling for non GEM drivers

Message ID 20210419092853.1605-1-christian.koenig@amd.com (mailing list archive)
State New, archived
Headers show
Series drm/ttm: remove special handling for non GEM drivers | expand

Commit Message

Christian König April 19, 2021, 9:28 a.m. UTC
vmwgfx is the only driver actually using this. Move the handling into
the driver instead.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c       | 11 -----------
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 10 ++++++++++
 include/drm/ttm/ttm_bo_api.h       | 19 -------------------
 3 files changed, 10 insertions(+), 30 deletions(-)

Comments

Huang Rui April 19, 2021, 1:19 p.m. UTC | #1
On Mon, Apr 19, 2021 at 05:28:53PM +0800, Christian König wrote:
> vmwgfx is the only driver actually using this. Move the handling into
> the driver instead.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Acked-by: Huang Rui <ray.huang@amd.com>

> ---
>  drivers/gpu/drm/ttm/ttm_bo.c       | 11 -----------
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 10 ++++++++++
>  include/drm/ttm/ttm_bo_api.h       | 19 -------------------
>  3 files changed, 10 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 80831df0ef61..38183e227116 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -460,8 +460,6 @@ static void ttm_bo_release(struct kref *kref)
>  
>  	atomic_dec(&ttm_glob.bo_count);
>  	dma_fence_put(bo->moving);
> -	if (!ttm_bo_uses_embedded_gem_object(bo))
> -		dma_resv_fini(&bo->base._resv);
>  	bo->destroy(bo);
>  }
>  
> @@ -1056,15 +1054,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>  	} else {
>  		bo->base.resv = &bo->base._resv;
>  	}
> -	if (!ttm_bo_uses_embedded_gem_object(bo)) {
> -		/*
> -		 * bo.base is not initialized, so we have to setup the
> -		 * struct elements we want use regardless.
> -		 */
> -		bo->base.size = size;
> -		dma_resv_init(&bo->base._resv);
> -		drm_vma_node_reset(&bo->base.vma_node);
> -	}
>  	atomic_inc(&ttm_glob.bo_count);
>  
>  	/*
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 50e529a01677..587314d57991 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -460,6 +460,7 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo)
>  	WARN_ON(vmw_bo->dirty);
>  	WARN_ON(!RB_EMPTY_ROOT(&vmw_bo->res_tree));
>  	vmw_bo_unmap(vmw_bo);
> +	dma_resv_fini(&bo->base._resv);
>  	kfree(vmw_bo);
>  }
>  
> @@ -512,6 +513,11 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
>  	if (unlikely(ret))
>  		goto error_free;
>  
> +
> +	bo->base.size = size;
> +	dma_resv_init(&bo->base._resv);
> +	drm_vma_node_reset(&bo->base.vma_node);
> +
>  	ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size,
>  				   ttm_bo_type_device, placement, 0,
>  				   &ctx, NULL, NULL, NULL);
> @@ -570,6 +576,10 @@ int vmw_bo_init(struct vmw_private *dev_priv,
>  	if (unlikely(ret))
>  		return ret;
>  
> +	vmw_bo->base.base.size = size;
> +	dma_resv_init(&vmw_bo->base.base._resv);
> +	drm_vma_node_reset(&vmw_bo->base.base.vma_node);
> +
>  	ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size,
>  				   ttm_bo_type_device, placement,
>  				   0, &ctx, NULL, NULL, bo_free);
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 3587f660e8f4..e88da481a976 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -562,25 +562,6 @@ ssize_t ttm_bo_io(struct ttm_device *bdev, struct file *filp,
>  int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
>  		   gfp_t gfp_flags);
>  
> -/**
> - * ttm_bo_uses_embedded_gem_object - check if the given bo uses the
> - * embedded drm_gem_object.
> - *
> - * Most ttm drivers are using gem too, so the embedded
> - * ttm_buffer_object.base will be initialized by the driver (before
> - * calling ttm_bo_init).  It is also possible to use ttm without gem
> - * though (vmwgfx does that).
> - *
> - * This helper will figure whenever a given ttm bo is a gem object too
> - * or not.
> - *
> - * @bo: The bo to check.
> - */
> -static inline bool ttm_bo_uses_embedded_gem_object(struct ttm_buffer_object *bo)
> -{
> -	return bo->base.dev != NULL;
> -}
> -
>  /**
>   * ttm_bo_pin - Pin the buffer object.
>   * @bo: The buffer object to pin
> -- 
> 2.25.1
>
Nirmoy April 19, 2021, 4:39 p.m. UTC | #2
Tested-by: Nirmoy Das <nirmoy.das@amd.com>

On 4/19/21 11:28 AM, Christian König wrote:
> vmwgfx is the only driver actually using this. Move the handling into
> the driver instead.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c       | 11 -----------
>   drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 10 ++++++++++
>   include/drm/ttm/ttm_bo_api.h       | 19 -------------------
>   3 files changed, 10 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 80831df0ef61..38183e227116 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -460,8 +460,6 @@ static void ttm_bo_release(struct kref *kref)
>   
>   	atomic_dec(&ttm_glob.bo_count);
>   	dma_fence_put(bo->moving);
> -	if (!ttm_bo_uses_embedded_gem_object(bo))
> -		dma_resv_fini(&bo->base._resv);
>   	bo->destroy(bo);
>   }
>   
> @@ -1056,15 +1054,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>   	} else {
>   		bo->base.resv = &bo->base._resv;
>   	}
> -	if (!ttm_bo_uses_embedded_gem_object(bo)) {
> -		/*
> -		 * bo.base is not initialized, so we have to setup the
> -		 * struct elements we want use regardless.
> -		 */
> -		bo->base.size = size;
> -		dma_resv_init(&bo->base._resv);
> -		drm_vma_node_reset(&bo->base.vma_node);
> -	}
>   	atomic_inc(&ttm_glob.bo_count);
>   
>   	/*
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 50e529a01677..587314d57991 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -460,6 +460,7 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo)
>   	WARN_ON(vmw_bo->dirty);
>   	WARN_ON(!RB_EMPTY_ROOT(&vmw_bo->res_tree));
>   	vmw_bo_unmap(vmw_bo);
> +	dma_resv_fini(&bo->base._resv);
>   	kfree(vmw_bo);
>   }
>   
> @@ -512,6 +513,11 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
>   	if (unlikely(ret))
>   		goto error_free;
>   
> +
> +	bo->base.size = size;
> +	dma_resv_init(&bo->base._resv);
> +	drm_vma_node_reset(&bo->base.vma_node);
> +
>   	ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size,
>   				   ttm_bo_type_device, placement, 0,
>   				   &ctx, NULL, NULL, NULL);
> @@ -570,6 +576,10 @@ int vmw_bo_init(struct vmw_private *dev_priv,
>   	if (unlikely(ret))
>   		return ret;
>   
> +	vmw_bo->base.base.size = size;
> +	dma_resv_init(&vmw_bo->base.base._resv);
> +	drm_vma_node_reset(&vmw_bo->base.base.vma_node);
> +
>   	ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size,
>   				   ttm_bo_type_device, placement,
>   				   0, &ctx, NULL, NULL, bo_free);
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 3587f660e8f4..e88da481a976 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -562,25 +562,6 @@ ssize_t ttm_bo_io(struct ttm_device *bdev, struct file *filp,
>   int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
>   		   gfp_t gfp_flags);
>   
> -/**
> - * ttm_bo_uses_embedded_gem_object - check if the given bo uses the
> - * embedded drm_gem_object.
> - *
> - * Most ttm drivers are using gem too, so the embedded
> - * ttm_buffer_object.base will be initialized by the driver (before
> - * calling ttm_bo_init).  It is also possible to use ttm without gem
> - * though (vmwgfx does that).
> - *
> - * This helper will figure whenever a given ttm bo is a gem object too
> - * or not.
> - *
> - * @bo: The bo to check.
> - */
> -static inline bool ttm_bo_uses_embedded_gem_object(struct ttm_buffer_object *bo)
> -{
> -	return bo->base.dev != NULL;
> -}
> -
>   /**
>    * ttm_bo_pin - Pin the buffer object.
>    * @bo: The buffer object to pin
Christian König April 22, 2021, 11:59 a.m. UTC | #3
Zack or Roland any objections to this?

There shouldn't be any functional change.

Thanks,
Christian.

Am 19.04.21 um 11:28 schrieb Christian König:
> vmwgfx is the only driver actually using this. Move the handling into
> the driver instead.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_bo.c       | 11 -----------
>   drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 10 ++++++++++
>   include/drm/ttm/ttm_bo_api.h       | 19 -------------------
>   3 files changed, 10 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 80831df0ef61..38183e227116 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -460,8 +460,6 @@ static void ttm_bo_release(struct kref *kref)
>   
>   	atomic_dec(&ttm_glob.bo_count);
>   	dma_fence_put(bo->moving);
> -	if (!ttm_bo_uses_embedded_gem_object(bo))
> -		dma_resv_fini(&bo->base._resv);
>   	bo->destroy(bo);
>   }
>   
> @@ -1056,15 +1054,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
>   	} else {
>   		bo->base.resv = &bo->base._resv;
>   	}
> -	if (!ttm_bo_uses_embedded_gem_object(bo)) {
> -		/*
> -		 * bo.base is not initialized, so we have to setup the
> -		 * struct elements we want use regardless.
> -		 */
> -		bo->base.size = size;
> -		dma_resv_init(&bo->base._resv);
> -		drm_vma_node_reset(&bo->base.vma_node);
> -	}
>   	atomic_inc(&ttm_glob.bo_count);
>   
>   	/*
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> index 50e529a01677..587314d57991 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
> @@ -460,6 +460,7 @@ void vmw_bo_bo_free(struct ttm_buffer_object *bo)
>   	WARN_ON(vmw_bo->dirty);
>   	WARN_ON(!RB_EMPTY_ROOT(&vmw_bo->res_tree));
>   	vmw_bo_unmap(vmw_bo);
> +	dma_resv_fini(&bo->base._resv);
>   	kfree(vmw_bo);
>   }
>   
> @@ -512,6 +513,11 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
>   	if (unlikely(ret))
>   		goto error_free;
>   
> +
> +	bo->base.size = size;
> +	dma_resv_init(&bo->base._resv);
> +	drm_vma_node_reset(&bo->base.vma_node);
> +
>   	ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size,
>   				   ttm_bo_type_device, placement, 0,
>   				   &ctx, NULL, NULL, NULL);
> @@ -570,6 +576,10 @@ int vmw_bo_init(struct vmw_private *dev_priv,
>   	if (unlikely(ret))
>   		return ret;
>   
> +	vmw_bo->base.base.size = size;
> +	dma_resv_init(&vmw_bo->base.base._resv);
> +	drm_vma_node_reset(&vmw_bo->base.base.vma_node);
> +
>   	ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size,
>   				   ttm_bo_type_device, placement,
>   				   0, &ctx, NULL, NULL, bo_free);
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 3587f660e8f4..e88da481a976 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -562,25 +562,6 @@ ssize_t ttm_bo_io(struct ttm_device *bdev, struct file *filp,
>   int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
>   		   gfp_t gfp_flags);
>   
> -/**
> - * ttm_bo_uses_embedded_gem_object - check if the given bo uses the
> - * embedded drm_gem_object.
> - *
> - * Most ttm drivers are using gem too, so the embedded
> - * ttm_buffer_object.base will be initialized by the driver (before
> - * calling ttm_bo_init).  It is also possible to use ttm without gem
> - * though (vmwgfx does that).
> - *
> - * This helper will figure whenever a given ttm bo is a gem object too
> - * or not.
> - *
> - * @bo: The bo to check.
> - */
> -static inline bool ttm_bo_uses_embedded_gem_object(struct ttm_buffer_object *bo)
> -{
> -	return bo->base.dev != NULL;
> -}
> -
>   /**
>    * ttm_bo_pin - Pin the buffer object.
>    * @bo: The buffer object to pin
Zack Rusin April 22, 2021, 2:53 p.m. UTC | #4
On 4/22/21 7:59 AM, Christian König wrote:
> Zack or Roland any objections to this?
> 
> There shouldn't be any functional change.
> 

Sorry, that looks good. I wanted us to add gem support for a while now, this just adds more reasons to do it for next merge window.

Reviewed-by: Zack Rusin <zackr@vmware.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 80831df0ef61..38183e227116 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -460,8 +460,6 @@  static void ttm_bo_release(struct kref *kref)
 
 	atomic_dec(&ttm_glob.bo_count);
 	dma_fence_put(bo->moving);
-	if (!ttm_bo_uses_embedded_gem_object(bo))
-		dma_resv_fini(&bo->base._resv);
 	bo->destroy(bo);
 }
 
@@ -1056,15 +1054,6 @@  int ttm_bo_init_reserved(struct ttm_device *bdev,
 	} else {
 		bo->base.resv = &bo->base._resv;
 	}
-	if (!ttm_bo_uses_embedded_gem_object(bo)) {
-		/*
-		 * bo.base is not initialized, so we have to setup the
-		 * struct elements we want use regardless.
-		 */
-		bo->base.size = size;
-		dma_resv_init(&bo->base._resv);
-		drm_vma_node_reset(&bo->base.vma_node);
-	}
 	atomic_inc(&ttm_glob.bo_count);
 
 	/*
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 50e529a01677..587314d57991 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -460,6 +460,7 @@  void vmw_bo_bo_free(struct ttm_buffer_object *bo)
 	WARN_ON(vmw_bo->dirty);
 	WARN_ON(!RB_EMPTY_ROOT(&vmw_bo->res_tree));
 	vmw_bo_unmap(vmw_bo);
+	dma_resv_fini(&bo->base._resv);
 	kfree(vmw_bo);
 }
 
@@ -512,6 +513,11 @@  int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
 	if (unlikely(ret))
 		goto error_free;
 
+
+	bo->base.size = size;
+	dma_resv_init(&bo->base._resv);
+	drm_vma_node_reset(&bo->base.vma_node);
+
 	ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size,
 				   ttm_bo_type_device, placement, 0,
 				   &ctx, NULL, NULL, NULL);
@@ -570,6 +576,10 @@  int vmw_bo_init(struct vmw_private *dev_priv,
 	if (unlikely(ret))
 		return ret;
 
+	vmw_bo->base.base.size = size;
+	dma_resv_init(&vmw_bo->base.base._resv);
+	drm_vma_node_reset(&vmw_bo->base.base.vma_node);
+
 	ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size,
 				   ttm_bo_type_device, placement,
 				   0, &ctx, NULL, NULL, bo_free);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 3587f660e8f4..e88da481a976 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -562,25 +562,6 @@  ssize_t ttm_bo_io(struct ttm_device *bdev, struct file *filp,
 int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
 		   gfp_t gfp_flags);
 
-/**
- * ttm_bo_uses_embedded_gem_object - check if the given bo uses the
- * embedded drm_gem_object.
- *
- * Most ttm drivers are using gem too, so the embedded
- * ttm_buffer_object.base will be initialized by the driver (before
- * calling ttm_bo_init).  It is also possible to use ttm without gem
- * though (vmwgfx does that).
- *
- * This helper will figure whenever a given ttm bo is a gem object too
- * or not.
- *
- * @bo: The bo to check.
- */
-static inline bool ttm_bo_uses_embedded_gem_object(struct ttm_buffer_object *bo)
-{
-	return bo->base.dev != NULL;
-}
-
 /**
  * ttm_bo_pin - Pin the buffer object.
  * @bo: The buffer object to pin