[PATCH] drm/ttm: remove special handling for non GEM drivers

[PATCH] drm/ttm: remove special handling for non GEM drivers

[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
-- 
2.25.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: remove special handling for non GEM drivers

[Huang Rui]

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
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: remove special handling for non GEM drivers

[Nirmoy]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: remove special handling for non GEM drivers

[Christian König]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: remove special handling for non GEM drivers

[Zack Rusin]

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>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: remove special handling for non GEM drivers

[Jamie Heilman]

Christian König wrote:
> vmwgfx is the only driver actually using this. Move the handling into
> the driver instead.

Maybe it isn't the only driver?  This commit broke video on my system
with a G86 [Quadro NVS 290].  Bisected to
d02117f8efaa5fbc37437df1ae955a147a2a424a, and reverting it restored
functionality.

dmesg of v5.14-rc2:

 nouveau 0000:01:00.0: vgaarb: deactivate vga console
 Console: switching to colour dummy device 80x25
 nouveau 0000:01:00.0: NVIDIA G86 (086f00a2)
 nouveau 0000:01:00.0: bios: version 60.86.6c.00.21
 nouveau 0000:01:00.0: bios: M0203T not found
 nouveau 0000:01:00.0: bios: M0203E not matched!
 nouveau 0000:01:00.0: fb: 256 MiB DDR2
 nouveau 0000:01:00.0: DRM: VRAM: 256 MiB
 nouveau 0000:01:00.0: DRM: GART: 1048576 MiB
 nouveau 0000:01:00.0: DRM: TMDS table version 2.0
 nouveau 0000:01:00.0: DRM: DCB version 4.0
 nouveau 0000:01:00.0: DRM: DCB outp 00: 02011300 00000028
 nouveau 0000:01:00.0: DRM: DCB outp 01: 01011302 00000010
 nouveau 0000:01:00.0: DRM: DCB outp 02: 01000310 00000028
 nouveau 0000:01:00.0: DRM: DCB outp 03: 02000312 00000010
 nouveau 0000:01:00.0: DRM: DCB conn 00: 2030
 nouveau 0000:01:00.0: DRM: DCB conn 01: 1130
 nouveau 0000:01:00.0: DRM: failed to initialise sync subsystem, -28
 nouveau: probe of 0000:01:00.0 failed with error -28

dmesg of v5.14-rc2 w/d02117f8efaa5fbc37437df1ae955a147a2a424a
reverted:

 nouveau 0000:01:00.0: vgaarb: deactivate vga console
 Console: switching to colour dummy device 80x25
 nouveau 0000:01:00.0: NVIDIA G86 (086f00a2)
 nouveau 0000:01:00.0: bios: version 60.86.6c.00.21
 nouveau 0000:01:00.0: bios: M0203T not found
 nouveau 0000:01:00.0: bios: M0203E not matched!
 nouveau 0000:01:00.0: fb: 256 MiB DDR2
 nouveau 0000:01:00.0: DRM: VRAM: 256 MiB
 nouveau 0000:01:00.0: DRM: GART: 1048576 MiB
 nouveau 0000:01:00.0: DRM: TMDS table version 2.0
 nouveau 0000:01:00.0: DRM: DCB version 4.0
 nouveau 0000:01:00.0: DRM: DCB outp 00: 02011300 00000028
 nouveau 0000:01:00.0: DRM: DCB outp 01: 01011302 00000010
 nouveau 0000:01:00.0: DRM: DCB outp 02: 01000310 00000028
 nouveau 0000:01:00.0: DRM: DCB outp 03: 02000312 00000010
 nouveau 0000:01:00.0: DRM: DCB conn 00: 2030
 nouveau 0000:01:00.0: DRM: DCB conn 01: 1130
 nouveau 0000:01:00.0: DRM: MM: using CRYPT for buffer copies
 nouveau 0000:01:00.0: DRM: allocated 1920x1200 fb: 0x50000, bo 00000000d0819d42
 fbcon: nouveau (fb0) is primary device
 Console: switching to colour frame buffer device 240x75
 nouveau 0000:01:00.0: [drm] fb0: nouveau frame buffer device
 [drm] Initialized nouveau 1.3.1 20120801 for 0000:01:00.0 on minor 0


> 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
> -- 
> 2.25.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jamie Heilman                     http://audible.transient.net/~jamie/

Re: [PATCH] drm/ttm: remove special handling for non GEM drivers

[Christian König]

This is a known issue and fixed by:

commit a3a9ee4b5254f212c2adaa8cd8ca03bfa112f49d
Author: Christian König <christian.koenig@amd.com>
Date:   Wed Jun 9 19:25:56 2021 +0200

     drm/nouveau: init the base GEM fields for internal BOs

     TTMs buffer objects are based on GEM objects for quite a while
     and rely on initializing those fields before initializing the TTM BO.

     Nouveau now doesn't init the GEM object for internally allocated BOs,
     so make sure that we at least initialize some necessary fields.

Regards,
Christian.

Am 21.07.21 um 14:57 schrieb Jamie Heilman:
> Christian König wrote:
>> vmwgfx is the only driver actually using this. Move the handling into
>> the driver instead.
> Maybe it isn't the only driver?  This commit broke video on my system
> with a G86 [Quadro NVS 290].  Bisected to
> d02117f8efaa5fbc37437df1ae955a147a2a424a, and reverting it restored
> functionality.
>
> dmesg of v5.14-rc2:
>
>   nouveau 0000:01:00.0: vgaarb: deactivate vga console
>   Console: switching to colour dummy device 80x25
>   nouveau 0000:01:00.0: NVIDIA G86 (086f00a2)
>   nouveau 0000:01:00.0: bios: version 60.86.6c.00.21
>   nouveau 0000:01:00.0: bios: M0203T not found
>   nouveau 0000:01:00.0: bios: M0203E not matched!
>   nouveau 0000:01:00.0: fb: 256 MiB DDR2
>   nouveau 0000:01:00.0: DRM: VRAM: 256 MiB
>   nouveau 0000:01:00.0: DRM: GART: 1048576 MiB
>   nouveau 0000:01:00.0: DRM: TMDS table version 2.0
>   nouveau 0000:01:00.0: DRM: DCB version 4.0
>   nouveau 0000:01:00.0: DRM: DCB outp 00: 02011300 00000028
>   nouveau 0000:01:00.0: DRM: DCB outp 01: 01011302 00000010
>   nouveau 0000:01:00.0: DRM: DCB outp 02: 01000310 00000028
>   nouveau 0000:01:00.0: DRM: DCB outp 03: 02000312 00000010
>   nouveau 0000:01:00.0: DRM: DCB conn 00: 2030
>   nouveau 0000:01:00.0: DRM: DCB conn 01: 1130
>   nouveau 0000:01:00.0: DRM: failed to initialise sync subsystem, -28
>   nouveau: probe of 0000:01:00.0 failed with error -28
>
> dmesg of v5.14-rc2 w/d02117f8efaa5fbc37437df1ae955a147a2a424a
> reverted:
>
>   nouveau 0000:01:00.0: vgaarb: deactivate vga console
>   Console: switching to colour dummy device 80x25
>   nouveau 0000:01:00.0: NVIDIA G86 (086f00a2)
>   nouveau 0000:01:00.0: bios: version 60.86.6c.00.21
>   nouveau 0000:01:00.0: bios: M0203T not found
>   nouveau 0000:01:00.0: bios: M0203E not matched!
>   nouveau 0000:01:00.0: fb: 256 MiB DDR2
>   nouveau 0000:01:00.0: DRM: VRAM: 256 MiB
>   nouveau 0000:01:00.0: DRM: GART: 1048576 MiB
>   nouveau 0000:01:00.0: DRM: TMDS table version 2.0
>   nouveau 0000:01:00.0: DRM: DCB version 4.0
>   nouveau 0000:01:00.0: DRM: DCB outp 00: 02011300 00000028
>   nouveau 0000:01:00.0: DRM: DCB outp 01: 01011302 00000010
>   nouveau 0000:01:00.0: DRM: DCB outp 02: 01000310 00000028
>   nouveau 0000:01:00.0: DRM: DCB outp 03: 02000312 00000010
>   nouveau 0000:01:00.0: DRM: DCB conn 00: 2030
>   nouveau 0000:01:00.0: DRM: DCB conn 01: 1130
>   nouveau 0000:01:00.0: DRM: MM: using CRYPT for buffer copies
>   nouveau 0000:01:00.0: DRM: allocated 1920x1200 fb: 0x50000, bo 00000000d0819d42
>   fbcon: nouveau (fb0) is primary device
>   Console: switching to colour frame buffer device 240x75
>   nouveau 0000:01:00.0: [drm] fb0: nouveau frame buffer device
>   [drm] Initialized nouveau 1.3.1 20120801 for 0000:01:00.0 on minor 0
>
>
>> 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
>> -- 
>> 2.25.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/ttm: remove special handling for non GEM drivers

[Jamie Heilman]

Christian König wrote:
> This is a known issue and fixed by:
>
> commit a3a9ee4b5254f212c2adaa8cd8ca03bfa112f49d
> Author: Christian König <christian.koenig@amd.com>
> Date:   Wed Jun 9 19:25:56 2021 +0200
> 
>     drm/nouveau: init the base GEM fields for internal BOs
> 
>     TTMs buffer objects are based on GEM objects for quite a while
>     and rely on initializing those fields before initializing the TTM BO.
> 
>     Nouveau now doesn't init the GEM object for internally allocated BOs,
>     so make sure that we at least initialize some necessary fields.

Ah, good deal.  Thanks

-- 
Jamie Heilman                     http://audible.transient.net/~jamie/