[PATCH 0/3] drm/vram-helper: Update GEM VRAM object creation

[PATCH 0/3] drm/vram-helper: Update GEM VRAM object creation

[Thomas Zimmermann]

This patchset updates drm_gem_vram_create() and related code. The
first patch fixes a double-free bug in the error path. Patch #2
simplifies the object creation. Patch #3 allow drivers to override
GEM functions.

Jia Yang (1):
  drm: fix double free for gbo in drm_gem_vram_init and
    drm_gem_vram_create

Thomas Zimmermann (2):
  drm/vram-helper: Integrate drm_gem_vram_init() into
    drm_gem_vram_create()
  drm/vram-helper: Set object function iff they are not provided by
    driver

 drivers/gpu/drm/drm_gem_vram_helper.c | 79 +++++++++++----------------
 1 file changed, 33 insertions(+), 46 deletions(-)

--
2.27.0

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

[PATCH 1/3] drm: fix double free for gbo in drm_gem_vram_init and drm_gem_vram_create

[Thomas Zimmermann]

From: Jia Yang <jiayang5@huawei.com>

I got a use-after-free report when doing some fuzz test:

If ttm_bo_init() fails, the "gbo" and "gbo->bo.base" will be
freed by ttm_buffer_object_destroy() in ttm_bo_init(). But
then drm_gem_vram_create() and drm_gem_vram_init() will free
"gbo" and "gbo->bo.base" again.

BUG: KMSAN: use-after-free in drm_vma_offset_remove+0xb3/0x150
CPU: 0 PID: 24282 Comm: syz-executor.1 Tainted: G    B   W         5.7.0-rc4-msan #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
Call Trace:
 __dump_stack
 dump_stack+0x1c9/0x220
 kmsan_report+0xf7/0x1e0
 __msan_warning+0x58/0xa0
 drm_vma_offset_remove+0xb3/0x150
 drm_gem_free_mmap_offset
 drm_gem_object_release+0x159/0x180
 drm_gem_vram_init
 drm_gem_vram_create+0x7c5/0x990
 drm_gem_vram_fill_create_dumb
 drm_gem_vram_driver_dumb_create+0x238/0x590
 drm_mode_create_dumb
 drm_mode_create_dumb_ioctl+0x41d/0x450
 drm_ioctl_kernel+0x5a4/0x710
 drm_ioctl+0xc6f/0x1240
 vfs_ioctl
 ksys_ioctl
 __do_sys_ioctl
 __se_sys_ioctl+0x2e9/0x410
 __x64_sys_ioctl+0x4a/0x70
 do_syscall_64+0xb8/0x160
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x4689b9
Code: fd e0 fa ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb e0 fa ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f368fa4dc98 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 000000000076bf00 RCX: 00000000004689b9
RDX: 0000000020000240 RSI: 00000000c02064b2 RDI: 0000000000000003
RBP: 0000000000000004 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00000000004d17e0 R14: 00007f368fa4e6d4 R15: 000000000076bf0c

Uninit was created at:
 kmsan_save_stack_with_flags
 kmsan_internal_poison_shadow+0x66/0xd0
 kmsan_slab_free+0x6e/0xb0
 slab_free_freelist_hook
 slab_free
 kfree+0x571/0x30a0
 drm_gem_vram_destroy
 ttm_buffer_object_destroy+0xc8/0x130
 ttm_bo_release
 kref_put
 ttm_bo_put+0x117d/0x23e0
 ttm_bo_init_reserved+0x11c0/0x11d0
 ttm_bo_init+0x289/0x3f0
 drm_gem_vram_init
 drm_gem_vram_create+0x775/0x990
 drm_gem_vram_fill_create_dumb
 drm_gem_vram_driver_dumb_create+0x238/0x590
 drm_mode_create_dumb
 drm_mode_create_dumb_ioctl+0x41d/0x450
 drm_ioctl_kernel+0x5a4/0x710
 drm_ioctl+0xc6f/0x1240
 vfs_ioctl
 ksys_ioctl
 __do_sys_ioctl
 __se_sys_ioctl+0x2e9/0x410
 __x64_sys_ioctl+0x4a/0x70
 do_syscall_64+0xb8/0x160
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

If ttm_bo_init() fails, the "gbo" will be freed by
ttm_buffer_object_destroy() in ttm_bo_init(). But then
drm_gem_vram_create() and drm_gem_vram_init() will free
"gbo" again.

Reported-by: Hulk Robot <hulkci@huawei.com>
Reported-by: butt3rflyh4ck <butterflyhuangxx@gmail.com>
Signed-off-by: Jia Yang <jiayang5@huawei.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 28 +++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index ad096600d89f..8dfb7458a34b 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -175,6 +175,10 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
 	}
 }
 
+/*
+ * Note that on error, drm_gem_vram_init will free the buffer object.
+ */
+
 static int drm_gem_vram_init(struct drm_device *dev,
 			     struct drm_gem_vram_object *gbo,
 			     size_t size, unsigned long pg_align)
@@ -184,15 +188,19 @@ static int drm_gem_vram_init(struct drm_device *dev,
 	int ret;
 	size_t acc_size;
 
-	if (WARN_ONCE(!vmm, "VRAM MM not initialized"))
+	if (WARN_ONCE(!vmm, "VRAM MM not initialized")) {
+		kfree(gbo);
 		return -EINVAL;
+	}
 	bdev = &vmm->bdev;
 
 	gbo->bo.base.funcs = &drm_gem_vram_object_funcs;
 
 	ret = drm_gem_object_init(dev, &gbo->bo.base, size);
-	if (ret)
+	if (ret) {
+		kfree(gbo);
 		return ret;
+	}
 
 	acc_size = ttm_bo_dma_acc_size(bdev, size, sizeof(*gbo));
 
@@ -203,13 +211,13 @@ static int drm_gem_vram_init(struct drm_device *dev,
 			  &gbo->placement, pg_align, false, acc_size,
 			  NULL, NULL, ttm_buffer_object_destroy);
 	if (ret)
-		goto err_drm_gem_object_release;
+		/*
+		 * A failing ttm_bo_init will call ttm_buffer_object_destroy
+		 * to release gbo->bo.base and kfree gbo.
+		 */
+		return ret;
 
 	return 0;
-
-err_drm_gem_object_release:
-	drm_gem_object_release(&gbo->bo.base);
-	return ret;
 }
 
 /**
@@ -243,13 +251,9 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
 
 	ret = drm_gem_vram_init(dev, gbo, size, pg_align);
 	if (ret < 0)
-		goto err_kfree;
+		return ERR_PTR(ret);
 
 	return gbo;
-
-err_kfree:
-	kfree(gbo);
-	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL(drm_gem_vram_create);
 
-- 
2.27.0

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

[PATCH 2/3] drm/vram-helper: Integrate drm_gem_vram_init() into drm_gem_vram_create()

[Thomas Zimmermann]

The drm_gem_vram_create() function is the only caller of the internal
helepr drm_gem_vram_init(). Streamline the code by putting all code into
the create function.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 75 ++++++++++-----------------
 1 file changed, 28 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 8dfb7458a34b..af767d3da5da 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -175,51 +175,6 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
 	}
 }
 
-/*
- * Note that on error, drm_gem_vram_init will free the buffer object.
- */
-
-static int drm_gem_vram_init(struct drm_device *dev,
-			     struct drm_gem_vram_object *gbo,
-			     size_t size, unsigned long pg_align)
-{
-	struct drm_vram_mm *vmm = dev->vram_mm;
-	struct ttm_bo_device *bdev;
-	int ret;
-	size_t acc_size;
-
-	if (WARN_ONCE(!vmm, "VRAM MM not initialized")) {
-		kfree(gbo);
-		return -EINVAL;
-	}
-	bdev = &vmm->bdev;
-
-	gbo->bo.base.funcs = &drm_gem_vram_object_funcs;
-
-	ret = drm_gem_object_init(dev, &gbo->bo.base, size);
-	if (ret) {
-		kfree(gbo);
-		return ret;
-	}
-
-	acc_size = ttm_bo_dma_acc_size(bdev, size, sizeof(*gbo));
-
-	gbo->bo.bdev = bdev;
-	drm_gem_vram_placement(gbo, TTM_PL_FLAG_VRAM | TTM_PL_FLAG_SYSTEM);
-
-	ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device,
-			  &gbo->placement, pg_align, false, acc_size,
-			  NULL, NULL, ttm_buffer_object_destroy);
-	if (ret)
-		/*
-		 * A failing ttm_bo_init will call ttm_buffer_object_destroy
-		 * to release gbo->bo.base and kfree gbo.
-		 */
-		return ret;
-
-	return 0;
-}
-
 /**
  * drm_gem_vram_create() - Creates a VRAM-backed GEM object
  * @dev:		the DRM device
@@ -235,7 +190,13 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
 						unsigned long pg_align)
 {
 	struct drm_gem_vram_object *gbo;
+	struct drm_vram_mm *vmm = dev->vram_mm;
+	struct ttm_bo_device *bdev;
 	int ret;
+	size_t acc_size;
+
+	if (WARN_ONCE(!vmm, "VRAM MM not initialized"))
+		return ERR_PTR(-EINVAL);
 
 	if (dev->driver->gem_create_object) {
 		struct drm_gem_object *gem =
@@ -249,8 +210,28 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
 			return ERR_PTR(-ENOMEM);
 	}
 
-	ret = drm_gem_vram_init(dev, gbo, size, pg_align);
-	if (ret < 0)
+	gbo->bo.base.funcs = &drm_gem_vram_object_funcs;
+
+	ret = drm_gem_object_init(dev, &gbo->bo.base, size);
+	if (ret) {
+		kfree(gbo);
+		return ERR_PTR(ret);
+	}
+
+	bdev = &vmm->bdev;
+	acc_size = ttm_bo_dma_acc_size(bdev, size, sizeof(*gbo));
+
+	gbo->bo.bdev = bdev;
+	drm_gem_vram_placement(gbo, TTM_PL_FLAG_VRAM | TTM_PL_FLAG_SYSTEM);
+
+	/*
+	 * A failing ttm_bo_init will call ttm_buffer_object_destroy
+	 * to release gbo->bo.base and kfree gbo.
+	 */
+	ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device,
+			  &gbo->placement, pg_align, false, acc_size,
+			  NULL, NULL, ttm_buffer_object_destroy);
+	if (ret)
 		return ERR_PTR(ret);
 
 	return gbo;
-- 
2.27.0

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

[PATCH 3/3] drm/vram-helper: Set object function iff they are not provided by driver

[Thomas Zimmermann]

Don't override the GEM object functions unconditionally. If the driver
sets the GEM functions, VRAM helpers will now them. The idea has been
taken from SHMEM helpers. If drivers need special versions of some of
the GEM functions, they can now override them.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index af767d3da5da..7194144610cb 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -190,6 +190,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
 						unsigned long pg_align)
 {
 	struct drm_gem_vram_object *gbo;
+	struct drm_gem_object *gem;
 	struct drm_vram_mm *vmm = dev->vram_mm;
 	struct ttm_bo_device *bdev;
 	int ret;
@@ -199,8 +200,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
 		return ERR_PTR(-EINVAL);
 
 	if (dev->driver->gem_create_object) {
-		struct drm_gem_object *gem =
-			dev->driver->gem_create_object(dev, size);
+		gem = dev->driver->gem_create_object(dev, size);
 		if (!gem)
 			return ERR_PTR(-ENOMEM);
 		gbo = drm_gem_vram_of_gem(gem);
@@ -208,11 +208,13 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
 		gbo = kzalloc(sizeof(*gbo), GFP_KERNEL);
 		if (!gbo)
 			return ERR_PTR(-ENOMEM);
+		gem = &gbo->bo.base;
 	}
 
-	gbo->bo.base.funcs = &drm_gem_vram_object_funcs;
+	if (!gem->funcs)
+		gem->funcs = &drm_gem_vram_object_funcs;
 
-	ret = drm_gem_object_init(dev, &gbo->bo.base, size);
+	ret = drm_gem_object_init(dev, gem, size);
 	if (ret) {
 		kfree(gbo);
 		return ERR_PTR(ret);
-- 
2.27.0

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

Re: [PATCH 2/3] drm/vram-helper: Integrate drm_gem_vram_init() into drm_gem_vram_create()

[Sam Ravnborg]

Hi Thomas.

On Tue, Jul 14, 2020 at 10:32:37AM +0200, Thomas Zimmermann wrote:
> The drm_gem_vram_create() function is the only caller of the internal
> helepr drm_gem_vram_init(). Streamline the code by putting all code into
helper
> the create function.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 75 ++++++++++-----------------
>  1 file changed, 28 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 8dfb7458a34b..af767d3da5da 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -175,51 +175,6 @@ static void drm_gem_vram_placement(struct drm_gem_vram_object *gbo,
>  	}
>  }
>  
> -/*
> - * Note that on error, drm_gem_vram_init will free the buffer object.
> - */
> -
> -static int drm_gem_vram_init(struct drm_device *dev,
> -			     struct drm_gem_vram_object *gbo,
> -			     size_t size, unsigned long pg_align)
> -{
> -	struct drm_vram_mm *vmm = dev->vram_mm;
> -	struct ttm_bo_device *bdev;
> -	int ret;
> -	size_t acc_size;
> -
> -	if (WARN_ONCE(!vmm, "VRAM MM not initialized")) {
> -		kfree(gbo);
> -		return -EINVAL;
> -	}
> -	bdev = &vmm->bdev;
> -
> -	gbo->bo.base.funcs = &drm_gem_vram_object_funcs;
> -
> -	ret = drm_gem_object_init(dev, &gbo->bo.base, size);
> -	if (ret) {
> -		kfree(gbo);
> -		return ret;
> -	}
> -
> -	acc_size = ttm_bo_dma_acc_size(bdev, size, sizeof(*gbo));
> -
> -	gbo->bo.bdev = bdev;
> -	drm_gem_vram_placement(gbo, TTM_PL_FLAG_VRAM | TTM_PL_FLAG_SYSTEM);
> -
> -	ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device,
> -			  &gbo->placement, pg_align, false, acc_size,
> -			  NULL, NULL, ttm_buffer_object_destroy);
> -	if (ret)
> -		/*
> -		 * A failing ttm_bo_init will call ttm_buffer_object_destroy
> -		 * to release gbo->bo.base and kfree gbo.
> -		 */
> -		return ret;
> -
> -	return 0;
> -}
> -
>  /**
>   * drm_gem_vram_create() - Creates a VRAM-backed GEM object
>   * @dev:		the DRM device
> @@ -235,7 +190,13 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>  						unsigned long pg_align)
>  {
>  	struct drm_gem_vram_object *gbo;
> +	struct drm_vram_mm *vmm = dev->vram_mm;
> +	struct ttm_bo_device *bdev;
>  	int ret;
> +	size_t acc_size;
> +
> +	if (WARN_ONCE(!vmm, "VRAM MM not initialized"))
> +		return ERR_PTR(-EINVAL);
>  
>  	if (dev->driver->gem_create_object) {
>  		struct drm_gem_object *gem =
> @@ -249,8 +210,28 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>  			return ERR_PTR(-ENOMEM);
>  	}
>  
> -	ret = drm_gem_vram_init(dev, gbo, size, pg_align);
> -	if (ret < 0)
> +	gbo->bo.base.funcs = &drm_gem_vram_object_funcs;
> +
> +	ret = drm_gem_object_init(dev, &gbo->bo.base, size);
> +	if (ret) {
> +		kfree(gbo);
> +		return ERR_PTR(ret);
> +	}
> +
> +	bdev = &vmm->bdev;
> +	acc_size = ttm_bo_dma_acc_size(bdev, size, sizeof(*gbo));
> +
> +	gbo->bo.bdev = bdev;
> +	drm_gem_vram_placement(gbo, TTM_PL_FLAG_VRAM | TTM_PL_FLAG_SYSTEM);
> +
> +	/*
> +	 * A failing ttm_bo_init will call ttm_buffer_object_destroy
> +	 * to release gbo->bo.base and kfree gbo.
> +	 */
> +	ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device,
> +			  &gbo->placement, pg_align, false, acc_size,
> +			  NULL, NULL, ttm_buffer_object_destroy);
> +	if (ret)
>  		return ERR_PTR(ret);
>  
>  	return gbo;
> -- 
> 2.27.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/vram-helper: Set object function iff they are not provided by driver

[Sam Ravnborg]

On Tue, Jul 14, 2020 at 10:32:38AM +0200, Thomas Zimmermann wrote:
> Don't override the GEM object functions unconditionally. If the driver
> sets the GEM functions, VRAM helpers will now them. The idea has been
s/now/own
> taken from SHMEM helpers. If drivers need special versions of some of
> the GEM functions, they can now override them.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index af767d3da5da..7194144610cb 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -190,6 +190,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>  						unsigned long pg_align)

The documentation of drm_gem_vram_create() could really use some love
here. It should document the behavior around gem_create_object(), and
the default allocation of drm_gem_vram_object with no drm_gem_object
assigned etc.

	Sam


>  {
>  	struct drm_gem_vram_object *gbo;
> +	struct drm_gem_object *gem;
>  	struct drm_vram_mm *vmm = dev->vram_mm;
>  	struct ttm_bo_device *bdev;
>  	int ret;
> @@ -199,8 +200,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>  		return ERR_PTR(-EINVAL);
>  
>  	if (dev->driver->gem_create_object) {
> -		struct drm_gem_object *gem =
> -			dev->driver->gem_create_object(dev, size);
> +		gem = dev->driver->gem_create_object(dev, size);
>  		if (!gem)
>  			return ERR_PTR(-ENOMEM);
>  		gbo = drm_gem_vram_of_gem(gem);
> @@ -208,11 +208,13 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>  		gbo = kzalloc(sizeof(*gbo), GFP_KERNEL);
>  		if (!gbo)
>  			return ERR_PTR(-ENOMEM);
> +		gem = &gbo->bo.base;
>  	}
>  
> -	gbo->bo.base.funcs = &drm_gem_vram_object_funcs;
> +	if (!gem->funcs)
> +		gem->funcs = &drm_gem_vram_object_funcs;
>  
> -	ret = drm_gem_object_init(dev, &gbo->bo.base, size);
> +	ret = drm_gem_object_init(dev, gem, size);
>  	if (ret) {
>  		kfree(gbo);
>  		return ERR_PTR(ret);
> -- 
> 2.27.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm/vram-helper: Set object function iff they are not provided by driver

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3031 bytes --]

Hi Sam

Am 16.07.20 um 22:11 schrieb Sam Ravnborg:
> On Tue, Jul 14, 2020 at 10:32:38AM +0200, Thomas Zimmermann wrote:
>> Don't override the GEM object functions unconditionally. If the driver
>> sets the GEM functions, VRAM helpers will now them. The idea has been
> s/now/own

Ooops, I forgot a word. This should have been 'will now use them'.

>> taken from SHMEM helpers. If drivers need special versions of some of
>> the GEM functions, they can now override them.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index af767d3da5da..7194144610cb 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -190,6 +190,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>>  						unsigned long pg_align)
> 
> The documentation of drm_gem_vram_create() could really use some love
> here. It should document the behavior around gem_create_object(), and
> the default allocation of drm_gem_vram_object with no drm_gem_object
> assigned etc.

Sure

Best regards
Thomas

> 
> 	Sam
> 
> 
>>  {
>>  	struct drm_gem_vram_object *gbo;
>> +	struct drm_gem_object *gem;
>>  	struct drm_vram_mm *vmm = dev->vram_mm;
>>  	struct ttm_bo_device *bdev;
>>  	int ret;
>> @@ -199,8 +200,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>>  		return ERR_PTR(-EINVAL);
>>  
>>  	if (dev->driver->gem_create_object) {
>> -		struct drm_gem_object *gem =
>> -			dev->driver->gem_create_object(dev, size);
>> +		gem = dev->driver->gem_create_object(dev, size);
>>  		if (!gem)
>>  			return ERR_PTR(-ENOMEM);
>>  		gbo = drm_gem_vram_of_gem(gem);
>> @@ -208,11 +208,13 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>>  		gbo = kzalloc(sizeof(*gbo), GFP_KERNEL);
>>  		if (!gbo)
>>  			return ERR_PTR(-ENOMEM);
>> +		gem = &gbo->bo.base;
>>  	}
>>  
>> -	gbo->bo.base.funcs = &drm_gem_vram_object_funcs;
>> +	if (!gem->funcs)
>> +		gem->funcs = &drm_gem_vram_object_funcs;
>>  
>> -	ret = drm_gem_object_init(dev, &gbo->bo.base, size);
>> +	ret = drm_gem_object_init(dev, gem, size);
>>  	if (ret) {
>>  		kfree(gbo);
>>  		return ERR_PTR(ret);
>> -- 
>> 2.27.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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